+Philippe for ISAPC On Thu, Nov 07, 2024 at 10:04:10AM +0300, Dmitry Frolov wrote: > Date: Thu, 7 Nov 2024 10:04:10 +0300 > From: Dmitry Frolov <fro...@swemel.ru> > Subject: [PATCH] hw/i386: fix NULL-dereference > > 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 <fro...@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); > + } > #ifdef CONFIG_IDE_ISA > if (!pcmc->pci_enabled) { > DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
In principle, I think the fix is right. Currently only ISAPC's pci_enabled is false. I think ISAPC shouldn't need nic, so it's safe. Is this right, Phil? :) The potential issue lies in pci_init_nic_devices() with "&bus->qbus". Although "bus" (which is pcibus here) is NULL, the compiler seems to optimize this, making &bus->qbus also NULL. Therefore, I did not encounter any errors when attempting to start ISAPC. Thanks, Zhao