| Message ID | 20241105171813.3031969-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/i386/pc: Don't try to init PCI NICs if there is no PCI bus | expand |
On Tue, 5 Nov 2024 at 17:18, Peter Maydell <peter.maydell@linaro.org> wrote: > > The 'isapc' machine type has no PCI bus, but pc_nic_init() still > calls pci_init_nic_devices() passing it a NULL bus pointer. This > causes the clang sanitizer to complain: > > $ ./build/clang/qemu-system-i386 -M isapc > ../../hw/pci/pci.c:1866:39: runtime error: member access within null pointer of type 'PCIBus' (aka 'struct PCIBus') > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/pci/pci.c:1866:39 in > > This is because pci_init_nic_devices() does > &bus->qbus > which is undefined behaviour on a NULL pointer even though we're not > actually dereferencing the pointer. (We don't actually crash as > a result, so if you aren't running a sanitizer build then there > are no user-visible effects.) > > Make pc_nic_init() avoid trying to initialize PCI NICs on a non-PCI > system. > > Cc: qemu-stable@nongnu.org > Fixes: 8d39f9ba14d64 ("hw/i386/pc: use qemu_get_nic_info() and pci_init_nic_devices()") > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This shows up if you run "make check" on a ubsan build. Incidentally, if pci_init_nic_devices() had done the more standard way to do "get a BusState* from a PCIBus*", i.e. use the QOM cast macro "BUS(bus)", that would also have avoided the UB (because QOM cast macros on NULL are valid and return NULL). But I figured not passing NULL in the first place was probably the intention rather than quietly handling NULL. thanks -- PMM
Am 5. November 2024 17:18:13 UTC schrieb Peter Maydell <peter.maydell@linaro.org>: >The 'isapc' machine type has no PCI bus, but pc_nic_init() still >calls pci_init_nic_devices() passing it a NULL bus pointer. This >causes the clang sanitizer to complain: > >$ ./build/clang/qemu-system-i386 -M isapc >../../hw/pci/pci.c:1866:39: runtime error: member access within null pointer of type 'PCIBus' (aka 'struct PCIBus') >SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/pci/pci.c:1866:39 in > >This is because pci_init_nic_devices() does > &bus->qbus >which is undefined behaviour on a NULL pointer even though we're not >actually dereferencing the pointer. (We don't actually crash as >a result, so if you aren't running a sanitizer build then there >are no user-visible effects.) > >Make pc_nic_init() avoid trying to initialize PCI NICs on a non-PCI >system. > >Cc: qemu-stable@nongnu.org >Fixes: 8d39f9ba14d64 ("hw/i386/pc: use qemu_get_nic_info() and pci_init_nic_devices()") >Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >--- >This shows up if you run "make check" on a ubsan build. >--- > hw/i386/pc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff --git a/hw/i386/pc.c b/hw/i386/pc.c >index 2047633e4cf..1af1a1a1823 100644 >--- a/hw/i386/pc.c >+++ b/hw/i386/pc.c >@@ -1251,7 +1251,9 @@ void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus) > } > > /* Anything remaining should be a PCI NIC */ >- pci_init_nic_devices(pci_bus, mc->default_nic); >+ if (pci_bus) { >+ pci_init_nic_devices(pci_bus, mc->default_nic); >+ } Reviewed-by: Bernhard Beschow <shentey@gmail.com> > > rom_reset_order_override(); > }
Queued, thanks. Paolo
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 2047633e4cf..1af1a1a1823 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1251,7 +1251,9 @@ void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus) } /* Anything remaining should be a PCI NIC */ - pci_init_nic_devices(pci_bus, mc->default_nic); + if (pci_bus) { + pci_init_nic_devices(pci_bus, mc->default_nic); + } rom_reset_order_override(); }
The 'isapc' machine type has no PCI bus, but pc_nic_init() still calls pci_init_nic_devices() passing it a NULL bus pointer. This causes the clang sanitizer to complain: $ ./build/clang/qemu-system-i386 -M isapc ../../hw/pci/pci.c:1866:39: runtime error: member access within null pointer of type 'PCIBus' (aka 'struct PCIBus') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/pci/pci.c:1866:39 in This is because pci_init_nic_devices() does &bus->qbus which is undefined behaviour on a NULL pointer even though we're not actually dereferencing the pointer. (We don't actually crash as a result, so if you aren't running a sanitizer build then there are no user-visible effects.) Make pc_nic_init() avoid trying to initialize PCI NICs on a non-PCI system. Cc: qemu-stable@nongnu.org Fixes: 8d39f9ba14d64 ("hw/i386/pc: use qemu_get_nic_info() and pci_init_nic_devices()") Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- This shows up if you run "make check" on a ubsan build. --- hw/i386/pc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)