All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: Fwd: QEMU | READ memory access in /hw/acpi/pcihp.c (#770)
       [not found] ` <37127769-a8d3-9d5f-5bfe-c0a0a8b9d3e2@redhat.com>
@ 2021-12-13 10:33   ` Michael S. Tsirkin
  2021-12-13 14:28     ` Igor Mammedov
  0 siblings, 1 reply; 2+ messages in thread
From: Michael S. Tsirkin @ 2021-12-13 10:33 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Igor Mammedov, qemu-devel

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);



^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: QEMU | READ memory access in /hw/acpi/pcihp.c (#770)
  2021-12-13 10:33   ` Fwd: QEMU | READ memory access in /hw/acpi/pcihp.c (#770) Michael S. Tsirkin
@ 2021-12-13 14:28     ` Igor Mammedov
  0 siblings, 0 replies; 2+ messages in thread
From: Igor Mammedov @ 2021-12-13 14:28 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Thomas Huth, qemu-devel

On Mon, 13 Dec 2021 05:33:43 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> 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?

It should fix the issue, would you like to send a formal patch?

(as for impact, it should not case issue in normal use-case,
max possible harm would be crashing qemu/VM if user has write
rights to pcihp IO window (typically restricted to superuser),
that's what test case/reproducer was doing)

> 
> 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);





^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-12-13 14:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <issue_98895819@gitlab.com>
     [not found] ` <37127769-a8d3-9d5f-5bfe-c0a0a8b9d3e2@redhat.com>
2021-12-13 10:33   ` Fwd: QEMU | READ memory access in /hw/acpi/pcihp.c (#770) Michael S. Tsirkin
2021-12-13 14:28     ` Igor Mammedov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.