[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Fwd: [PATCH] QEMU patch for PCI handling bug (invalid f
From: |
Matthias Weckbecker |
Subject: |
Re: [Qemu-devel] Fwd: [PATCH] QEMU patch for PCI handling bug (invalid free) |
Date: |
Tue, 11 Dec 2018 08:20:27 +0100 |
User-agent: |
Mutt/1.9.4 (2018-02-28) |
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 <address@hidden> -----
>
> (gdb) bt
> #0 __GI_raise (address@hidden) at ../sysdeps/unix/sysv/linux/raise.c:51
> #1 0x00007fde12dfc801 in __GI_abort () at abort.c:79
> #2 0x00007fde12e45897 in __libc_message (address@hidden, address@hidden
> "%s\n") at ../sysdeps/posix/libc_fatal.c:181
> #3 0x00007fde12e4c90a in malloc_printerr (address@hidden "corrupted size vs.
> prev_size") at malloc.c:5350
> #4 0x00007fde12e4cb0c in malloc_consolidate (address@hidden <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 <address@hidden>
> 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 <address@hidden>
> ---
> 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 -----
0001-hw-pci-bridge-Fix-invalid-free.patch
Description: Text document