* [Qemu-devel] Fwd: [PATCH] QEMU patch for PCI handling bug (invalid free)
@ 2018-12-10 14:56 Kevin Wolf
2018-12-11 7:20 ` Matthias Weckbecker
0 siblings, 1 reply; 3+ messages in thread
From: Kevin Wolf @ 2018-12-10 14:56 UTC (permalink / raw)
To: qemu-devel; +Cc: matthias, mst, marcel.apfelbaum
Am 10.12.2018 um 14:38 hat Matthias Weckbecker geschrieben:
> Hi Kevin,
>
> I'm attaching a patch for qemu. Read below for details.
>
> There's a bug in qemu in the PCI bridge handling that can be triggered when
> following the steps below:
>
> 1) Create some VM (e.g. w/ virsh define)
> 2) Ensure there is a PCI bridge attached, e.g. w/ libvirt like this:
>
> <controller type='pci' index='1' model='pci-bridge'>
> <model name='pci-bridge'/>
> <target chassisNr='1'/>
> <alias name='pci'/>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> </controller>
>
> 3) Take a snapshot while it's *running*, like this:
>
> % virsh start mips64el-vm
> % virsh snapshot-create-as mips64el-vm mips64el-snap
>
> 4) Destroy the VM and revert from snapshot:
>
> % virsh destroy mips64el-vm
> % virsh snapshot-revert mips64el-vm mips64el-snap --running
> error: internal error: process exited while connecting to monitor: corrupted size vs. prev_size
>
> 5) qemu-system-mips64el crashes
>
> The attached patch resolves the issue. Can you maybe review+commit, please?
Hi Matthias,
thanks for sending the patch. The next step is that it needs to be
reviewed on the qemu-devel mailing list (CCed) and then the PCI
subsystem maintainers (Michael and Marcel, CCed as well) can commit it.
Maybe some of the explanation above should actually be moved to the
commit message of the patch itself?
Kevin
----- Forwarded message from Matthias Weckbecker <matthias@weckbecker.name> -----
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007fde12dfc801 in __GI_abort () at abort.c:79
#2 0x00007fde12e45897 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7fde12f72b9a "%s\n") at ../sysdeps/posix/libc_fatal.c:181
#3 0x00007fde12e4c90a in malloc_printerr (str=str@entry=0x7fde12f70c9d "corrupted size vs. prev_size") at malloc.c:5350
#4 0x00007fde12e4cb0c in malloc_consolidate (av=av@entry=0x7fde131a7c40 <main_arena>) at malloc.c:4456
#5 0x00007fde12e5403b in _int_free (have_lock=0, p=<optimized out>, av=0x7fde131a7c40 <main_arena>) at malloc.c:4362
#6 __GI___libc_free (mem=0x55f089173c20) at malloc.c:3124
#7 0x000055f086c85062 in phys_section_destroy (mr=0x55f089173c20) at ./exec.c:1317
#8 phys_sections_free (map=0x55f0890f1560) at ./exec.c:1325
#9 address_space_dispatch_free (d=0x55f0890f1550) at ./exec.c:2777
#10 0x000055f086cc0509 in flatview_destroy (view=0x55f088a5caf0) at ./memory.c:301
#11 0x000055f087031dc0 in call_rcu_thread (opaque=<optimized out>) at ./util/rcu.c:272
#12 0x00007fde131b46db in start_thread (arg=0x7fde0aa39700) at pthread_create.c:463
#13 0x00007fde12edd88f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
==13641== Thread 2: [0/5041]
==13641== Invalid read of size 8
==13641== at 0x42755B: memory_region_unref (memory.c:1749)
==13641== by 0x42755B: flatview_destroy (memory.c:307)
==13641== by 0x798DBF: call_rcu_thread (rcu.c:272)
==13641== by 0x97BF6DA: start_thread (pthread_create.c:463)
==13641== by 0x9AF888E: clone (clone.S:95)
==13641== Address 0x408e4670 is 64 bytes inside a block of size 1,440 free'd
==13641== at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13641== by 0x5E988B: get_pci_config_device (pci.c:491)
==13641== by 0x660069: vmstate_load_state (vmstate.c:140)
==13641== by 0x660236: vmstate_load_state (vmstate.c:137)
==13641== by 0x659994: vmstate_load (savevm.c:748)
==13641== by 0x65A7B1: qemu_loadvm_section_start_full.isra.11 (savevm.c:1918)
==13641== by 0x65AA67: qemu_loadvm_state_main.isra.13 (savevm.c:2013)
==13641== by 0x65D7CE: qemu_loadvm_state (savevm.c:2092)
==13641== by 0x65E40D: load_snapshot (savevm.c:2406)
==13641== by 0x3E28C2: main (vl.c:4918)
==13641== Block was alloc'd at
==13641== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13641== by 0x8D35A28: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.5600.3)
==13641== by 0x5ECA8A: pci_bridge_region_init (pci_bridge.c:187)
==13641== by 0x5ECEFC: pci_bridge_initfn (pci_bridge.c:384)
==13641== by 0x5E654F: pci_bridge_dev_realize (pci_bridge_dev.c:59)
==13641== by 0x5EBED0: pci_qdev_realize (pci.c:2034)
==13641== by 0x5742D8: device_set_realized (qdev.c:914)
==13641== by 0x6B8F96: property_set_bool (object.c:1906)
==13641== by 0x6BD11E: object_property_set_qobject (qom-qobject.c:27)
==13641== by 0x6BAD7F: object_property_set_bool (object.c:1171)
==13641== by 0x4FA75D: qdev_device_add (qdev-monitor.c:629)
==13641== by 0x4FCD36: device_init_func (vl.c:2432)
==13641==
>From 8229eb9cb97a1806e264290e2935261bf23c7f34 Mon Sep 17 00:00:00 2001
From: Matthias Weckbecker <matthias@weckbecker.name>
Date: Mon, 10 Dec 2018 14:00:48 +0100
Subject: [PATCH] hw/pci-bridge: Fix invalid free()
When loadvm'ing a *running* snapshot qemu crashes due to an invalid
free. It's fortunately caught early by glibc heap memory corruption
protection and qemu gets killed with SIGABRT.
This commit fixes the issue.
Signed-off-by: Matthias Weckbecker <matthias@weckbecker.name>
---
hw/pci/pci_bridge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index ee9dff2d3a..b9143ac88b 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -241,9 +241,9 @@ void pci_bridge_update_mappings(PCIBridge *br)
* while another accesses an unaffected region. */
memory_region_transaction_begin();
pci_bridge_region_del(br, br->windows);
+ pci_bridge_region_cleanup(br, w);
br->windows = pci_bridge_region_init(br);
memory_region_transaction_commit();
- pci_bridge_region_cleanup(br, w);
}
/* default write_config function for PCI-to-PCI bridge */
--
2.11.0
----- End forwarded message -----
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] Fwd: [PATCH] QEMU patch for PCI handling bug (invalid free)
2018-12-10 14:56 [Qemu-devel] Fwd: [PATCH] QEMU patch for PCI handling bug (invalid free) Kevin Wolf
@ 2018-12-11 7:20 ` Matthias Weckbecker
2018-12-11 14:24 ` Eric Blake
0 siblings, 1 reply; 3+ messages in thread
From: Matthias Weckbecker @ 2018-12-11 7:20 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, mst, marcel.apfelbaum
[-- Attachment #1: Type: text/plain, Size: 6585 bytes --]
On Mon, Dec 10, 2018 at 03:56:53PM +0100, Kevin Wolf wrote:
> Am 10.12.2018 um 14:38 hat Matthias Weckbecker geschrieben:
> > Hi Kevin,
> >
> > I'm attaching a patch for qemu. Read below for details.
> >
> > There's a bug in qemu in the PCI bridge handling that can be triggered when
> > following the steps below:
> >
> > 1) Create some VM (e.g. w/ virsh define)
> > 2) Ensure there is a PCI bridge attached, e.g. w/ libvirt like this:
> >
> > <controller type='pci' index='1' model='pci-bridge'>
> > <model name='pci-bridge'/>
> > <target chassisNr='1'/>
> > <alias name='pci'/>
> > <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> > </controller>
> >
> > 3) Take a snapshot while it's *running*, like this:
> >
> > % virsh start mips64el-vm
> > % virsh snapshot-create-as mips64el-vm mips64el-snap
> >
> > 4) Destroy the VM and revert from snapshot:
> >
> > % virsh destroy mips64el-vm
> > % virsh snapshot-revert mips64el-vm mips64el-snap --running
> > error: internal error: process exited while connecting to monitor: corrupted size vs. prev_size
> >
> > 5) qemu-system-mips64el crashes
> >
> > The attached patch resolves the issue. Can you maybe review+commit, please?
>
> Hi Matthias,
>
Hi Kevin,
> thanks for sending the patch. The next step is that it needs to be
> reviewed on the qemu-devel mailing list (CCed) and then the PCI
> subsystem maintainers (Michael and Marcel, CCed as well) can commit it.
>
thank you for looking into it and forwarding it.
> Maybe some of the explanation above should actually be moved to the
> commit message of the patch itself?
>
yes, I agree. I have --amend'ed my commit message and re-attached it.
> Kevin
Thanks,
Matthias
>
> ----- Forwarded message from Matthias Weckbecker <matthias@weckbecker.name> -----
>
> (gdb) bt
> #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
> #1 0x00007fde12dfc801 in __GI_abort () at abort.c:79
> #2 0x00007fde12e45897 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7fde12f72b9a "%s\n") at ../sysdeps/posix/libc_fatal.c:181
> #3 0x00007fde12e4c90a in malloc_printerr (str=str@entry=0x7fde12f70c9d "corrupted size vs. prev_size") at malloc.c:5350
> #4 0x00007fde12e4cb0c in malloc_consolidate (av=av@entry=0x7fde131a7c40 <main_arena>) at malloc.c:4456
> #5 0x00007fde12e5403b in _int_free (have_lock=0, p=<optimized out>, av=0x7fde131a7c40 <main_arena>) at malloc.c:4362
> #6 __GI___libc_free (mem=0x55f089173c20) at malloc.c:3124
> #7 0x000055f086c85062 in phys_section_destroy (mr=0x55f089173c20) at ./exec.c:1317
> #8 phys_sections_free (map=0x55f0890f1560) at ./exec.c:1325
> #9 address_space_dispatch_free (d=0x55f0890f1550) at ./exec.c:2777
> #10 0x000055f086cc0509 in flatview_destroy (view=0x55f088a5caf0) at ./memory.c:301
> #11 0x000055f087031dc0 in call_rcu_thread (opaque=<optimized out>) at ./util/rcu.c:272
> #12 0x00007fde131b46db in start_thread (arg=0x7fde0aa39700) at pthread_create.c:463
> #13 0x00007fde12edd88f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
>
> ==13641== Thread 2: [0/5041]
> ==13641== Invalid read of size 8
> ==13641== at 0x42755B: memory_region_unref (memory.c:1749)
> ==13641== by 0x42755B: flatview_destroy (memory.c:307)
> ==13641== by 0x798DBF: call_rcu_thread (rcu.c:272)
> ==13641== by 0x97BF6DA: start_thread (pthread_create.c:463)
> ==13641== by 0x9AF888E: clone (clone.S:95)
> ==13641== Address 0x408e4670 is 64 bytes inside a block of size 1,440 free'd
> ==13641== at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==13641== by 0x5E988B: get_pci_config_device (pci.c:491)
> ==13641== by 0x660069: vmstate_load_state (vmstate.c:140)
> ==13641== by 0x660236: vmstate_load_state (vmstate.c:137)
> ==13641== by 0x659994: vmstate_load (savevm.c:748)
> ==13641== by 0x65A7B1: qemu_loadvm_section_start_full.isra.11 (savevm.c:1918)
> ==13641== by 0x65AA67: qemu_loadvm_state_main.isra.13 (savevm.c:2013)
> ==13641== by 0x65D7CE: qemu_loadvm_state (savevm.c:2092)
> ==13641== by 0x65E40D: load_snapshot (savevm.c:2406)
> ==13641== by 0x3E28C2: main (vl.c:4918)
> ==13641== Block was alloc'd at
> ==13641== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==13641== by 0x8D35A28: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.5600.3)
> ==13641== by 0x5ECA8A: pci_bridge_region_init (pci_bridge.c:187)
> ==13641== by 0x5ECEFC: pci_bridge_initfn (pci_bridge.c:384)
> ==13641== by 0x5E654F: pci_bridge_dev_realize (pci_bridge_dev.c:59)
> ==13641== by 0x5EBED0: pci_qdev_realize (pci.c:2034)
> ==13641== by 0x5742D8: device_set_realized (qdev.c:914)
> ==13641== by 0x6B8F96: property_set_bool (object.c:1906)
> ==13641== by 0x6BD11E: object_property_set_qobject (qom-qobject.c:27)
> ==13641== by 0x6BAD7F: object_property_set_bool (object.c:1171)
> ==13641== by 0x4FA75D: qdev_device_add (qdev-monitor.c:629)
> ==13641== by 0x4FCD36: device_init_func (vl.c:2432)
> ==13641==
>
>
> From 8229eb9cb97a1806e264290e2935261bf23c7f34 Mon Sep 17 00:00:00 2001
> From: Matthias Weckbecker <matthias@weckbecker.name>
> Date: Mon, 10 Dec 2018 14:00:48 +0100
> Subject: [PATCH] hw/pci-bridge: Fix invalid free()
>
> When loadvm'ing a *running* snapshot qemu crashes due to an invalid
> free. It's fortunately caught early by glibc heap memory corruption
> protection and qemu gets killed with SIGABRT.
>
> This commit fixes the issue.
>
> Signed-off-by: Matthias Weckbecker <matthias@weckbecker.name>
> ---
> hw/pci/pci_bridge.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> index ee9dff2d3a..b9143ac88b 100644
> --- a/hw/pci/pci_bridge.c
> +++ b/hw/pci/pci_bridge.c
> @@ -241,9 +241,9 @@ void pci_bridge_update_mappings(PCIBridge *br)
> * while another accesses an unaffected region. */
> memory_region_transaction_begin();
> pci_bridge_region_del(br, br->windows);
> + pci_bridge_region_cleanup(br, w);
> br->windows = pci_bridge_region_init(br);
> memory_region_transaction_commit();
> - pci_bridge_region_cleanup(br, w);
> }
>
> /* default write_config function for PCI-to-PCI bridge */
> --
> 2.11.0
>
> ----- End forwarded message -----
[-- Attachment #2: 0001-hw-pci-bridge-Fix-invalid-free.patch --]
[-- Type: text/plain, Size: 1362 bytes --]
>From 3f5ea73db8f781a43b0af0d3855cec604ba0434e Mon Sep 17 00:00:00 2001
From: Matthias Weckbecker <matthias@weckbecker.name>
Date: Mon, 10 Dec 2018 14:00:48 +0100
Subject: [PATCH] hw/pci-bridge: Fix invalid free()
When loadvm'ing a *running* snapshot qemu crashes due to an invalid
free. It's fortunately caught early by glibc heap memory corruption
protection and qemu gets killed with SIGABRT.
Steps to reproduce:
1) Create VM (e.g w/ virsh define)
2) Start the VM and take a snapshot while it's running and having a
PCI bridge attached
3) Destroy the VM and revert the running snapshot.
This commit fixes the issue.
Signed-off-by: Matthias Weckbecker <matthias@weckbecker.name>
---
hw/pci/pci_bridge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index ee9dff2d3a..b9143ac88b 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -241,9 +241,9 @@ void pci_bridge_update_mappings(PCIBridge *br)
* while another accesses an unaffected region. */
memory_region_transaction_begin();
pci_bridge_region_del(br, br->windows);
+ pci_bridge_region_cleanup(br, w);
br->windows = pci_bridge_region_init(br);
memory_region_transaction_commit();
- pci_bridge_region_cleanup(br, w);
}
/* default write_config function for PCI-to-PCI bridge */
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] Fwd: [PATCH] QEMU patch for PCI handling bug (invalid free)
2018-12-11 7:20 ` Matthias Weckbecker
@ 2018-12-11 14:24 ` Eric Blake
0 siblings, 0 replies; 3+ messages in thread
From: Eric Blake @ 2018-12-11 14:24 UTC (permalink / raw)
To: Matthias Weckbecker, Kevin Wolf; +Cc: qemu-devel, mst
On 12/11/18 1:20 AM, Matthias Weckbecker wrote:
>> Maybe some of the explanation above should actually be moved to the
>> commit message of the patch itself?
>>
>
> yes, I agree. I have --amend'ed my commit message and re-attached it.
It's best to resend the patch as a new top-level thread with v2 in the
title; 'git send-email -v2' can do that.
More patch submission hints:
https://wiki.qemu.org/Contribute/SubmitAPatch
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-12-11 14:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-10 14:56 [Qemu-devel] Fwd: [PATCH] QEMU patch for PCI handling bug (invalid free) Kevin Wolf
2018-12-11 7:20 ` Matthias Weckbecker
2018-12-11 14:24 ` Eric Blake
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.