Re: Fwd: QEMU | READ memory access in /hw/acpi/pcihp.c (#770)

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Dec 13, 2021 at 08:43:55AM +0100, Thomas Huth wrote:
> 
>  Hi Michael, hi Igor,
> 
> just FYI, a crash has been reported in the ACPI code ...
> by the way, do you have a gitlab account, so you could be put on CC: for
> such bugs there, too?
> 
>  Regards,
>   Thomas
> 
> 
> -------- Forwarded Message --------
> Subject: 	QEMU | READ memory access in /hw/acpi/pcihp.c (#770)
> Date: 	Sun, 12 Dec 2021 15:03:50 +0000
> From: 	LucasLeeNDS (@LucasLeeNDS) <gitlab@mg.gitlab.com>
> Reply-To: 	QEMU / QEMU
> <incoming+b01bab3884ecadb087cf49724e7b33aa@incoming.gitlab.com>
> To: 	thuth@redhat.com
> 
> 
> 
> GitLab
> 
> LucasLeeNDS <https://gitlab.com/LucasLeeNDS> created an issue: #770
> <https://gitlab.com/qemu-project/qemu/-/issues/770>
> 
> Hello qemu team, An invalid pointer initialization issue was found in
> /hw/acpi/pcihp.c:470:9 of QEMU in versions 6.2.0-rc2.
> 
> *Reproducer*
> 
> |cat << EOF | ./qemu-system-i386 \ -M pc -nodefaults -netdev user,id=user0
> -device virtio-net,netdev=user0 \ -qtest stdio outl 0xcf8 0x80000b00 inw
> 0xcfc outl 0xcf8 0x80000b04 inw 0xcfc outl 0xcf8 0x80000b04 outw 0xcfc 0x7
> outl 0xcf8 0x80000b04 inw 0xcfc outl 0xcf8 0x80000000 inw 0xcfc outl 0xcf8
> 0x80000004 inw 0xcfc outl 0xcf8 0x80000004 outw 0xcfc 0x7 outl 0xcf8
> 0x80000004 inw 0xcfc outl 0xcf8 0x80000800 inw 0xcfc outl 0xcf8 0x80000804
> inw 0xcfc outl 0xcf8 0x80000804 outw 0xcfc 0x7 outl 0xcf8 0x80000804 inw
> 0xcfc outl 0xcf8 0x80000900 inw 0xcfc outl 0xcf8 0x80000920 outl 0xcfc
> 0xffffffff outl 0xcf8 0x80000920 inl 0xcfc outl 0xcf8 0x80000920 outl 0xcfc
> 0xc001 outl 0xcf8 0x80000904 inw 0xcfc outl 0xcf8 0x80000904 outw 0xcfc 0x7
> outl 0xcf8 0x80000904 inw 0xcfc outl 0xcf8 0x80001000 inw 0xcfc outl 0xcf8
> 0x80001010 outl 0xcfc 0xffffffff outl 0xcf8 0x80001010 inl 0xcfc outl 0xcf8
> 0x80001010 outl 0xcfc 0xc021 outl 0xcf8 0x80001014 outl 0xcfc 0xffffffff
> outl 0xcf8 0x80001014 inl 0xcfc outl 0xcf8 0x80001014 outl 0xcfc 0xe0000000
> outl 0xcf8 0x80001020 outl 0xcfc 0xffffffff outl 0xcf8 0x80001020 inl 0xcfc
> outl 0xcf8 0x80001020 outl 0xcfc 0xe0004000 outl 0xcf8 0x80001004 inw 0xcfc
> outl 0xcf8 0x80001004 outw 0xcfc 0x7 outl 0xcf8 0x80001004 inw 0xcfc
> clock_step outl 0xae10 0x15 outl 0xae10 0x585a5564 outl 0xae10 0x15 outl
> 0xcf8 0x80000b06 outl 0xcfc 0xdd58fb5a outl 0xae14 0x64296572 clock_step
> outl 0xae10 0x15 outl 0xae10 0x585a5564 outl 0xae10 0x15 outl 0xcf8
> 0x80000b06 outl 0xcfc 0xdd58fb5a outl 0xae14 0x64296572 EOF|
> 
> *Stack-Trace*
> 
> |AddressSanitizer:DEADLYSIGNAL
> =================================================================
> ==4191==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000050 (pc
> 0x564df8697958 bp 0x7ffe620c13f0 sp 0x7ffe620c12a0 T0) ==4191==The signal is
> caused by a READ memory access. ==4191==Hint: address points to the zero
> page. #0 0x564df8697958 in pci_write
> /home/test/Desktop/qemu-6.2.0-rc2/build/../hw/acpi/pcihp.c:470:9 #1
> 0x564df941eb3c in memory_region_write_accessor

So it's this line:

        QTAILQ_FOREACH_SAFE(kid, &bus->qbus.children, sibling, next) {

in pci_write

probably because we supplied an invalid selector so bus is NULL?

Just checking that should do the job I think ...
Igor, what do you think?

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 30405b5113..a5e182dd3a 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -491,6 +491,9 @@ static void pci_write(void *opaque, hwaddr addr, uint64_t data,
         }
 
         bus = acpi_pcihp_find_hotplug_bus(s, s->hotplug_select);
+        if (!bus) {
+            break;
+        }
         QTAILQ_FOREACH_SAFE(kid, &bus->qbus.children, sibling, next) {
             Object *o = OBJECT(kid->child);
             PCIDevice *dev = PCI_DEVICE(o);