Re: [PATCH] hw/i386: fix NULL-dereference

[Bernhard Beschow]

Am 7. November 2024 09:19:05 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>
>
>Am 7. November 2024 07:04:10 UTC schrieb Dmitry Frolov <frolov@swemel.ru>:
>>If pcmc->pci_enabled is false, pcms->pcibus is NULL and is passed
>>to pc_nic_init() where it is being dereferenced.
>>
>>Found making check with enabled sanitizers.
>>
>>Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
>>---
>> hw/i386/pc_piix.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>>index 2bf6865d40..2a92d2dbb7 100644
>>--- a/hw/i386/pc_piix.c
>>+++ b/hw/i386/pc_piix.c
>>@@ -313,9 +313,9 @@ static void pc_init1(MachineState *machine, const char 
>>*pci_type)
>>     /* init basic PC hardware */
>>     pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
>>                          !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
>>-
>>-    pc_nic_init(pcmc, isa_bus, pcms->pcibus);
>>-
>>+    if (pcmc->pci_enabled) {
>>+        pc_nic_init(pcmc, isa_bus, pcms->pcibus);
>>+    }
>
>Since pc_nic_init() is passed both an ISA and a PCI bus I think the NULL 
>dereference should be fixed there. Moreover, if pc_nic_init() is only invoked 
>when there is a PCI bus, the "isapc" machine won't have a nic at all.
>
>How is this patch related to 
>20241105171813.3031969-1-peter.maydell@linaro.org">https://patchew.org/QEMU/20241105171813.3031969-1-peter.maydell@linaro.org ? 
>That is, do we need both patches to really fix the issue?

I've had a closer look at the code and Peter's patch nails the problem.

Best regards,
Bernhard

>
>Best regards,
>Bernhard
>
>> #ifdef CONFIG_IDE_ISA
>>     if (!pcmc->pci_enabled) {
>>         DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];