[Qemu-devel] Memory corruption with secondary-vga (was: [PATCH v8 5/9] secondary-vga: properly close QemuConsole on unplug)

[Thomas Huth]

On 06.03.2018 12:34, Gerd Hoffmann wrote:
> Using the new graphic_console_close() function.
> 
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  hw/display/vga-pci.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
> index 1674bd3581..f312930664 100644
> --- a/hw/display/vga-pci.c
> +++ b/hw/display/vga-pci.c
> @@ -292,6 +292,14 @@ static void pci_secondary_vga_realize(PCIDevice *dev, 
> Error **errp)
>      pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
>  }
>  
> +static void pci_secondary_vga_exit(PCIDevice *dev)
> +{
> +    PCIVGAState *d = PCI_VGA(dev);
> +    VGACommonState *s = &d->vga;
> +
> +    graphic_console_close(s->con);
> +}
> +
>  static void pci_secondary_vga_init(Object *obj)
>  {
>      /* Expose framebuffer byteorder via QOM */
> @@ -361,6 +369,7 @@ static void secondary_class_init(ObjectClass *klass, void 
> *data)
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
>      k->realize = pci_secondary_vga_realize;
> +    k->exit = pci_secondary_vga_exit;

 Hi Gerd,

not sure what is exactly happening here, but this patch introduced
a memory corruption problem. I can trigger it like this:

$ valgrind mips-softmmu/qemu-system-mips -accel qtest -monitor stdio
==12739== Memcheck, a memory error detector
==12739== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==12739== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==12739== Command: mips-softmmu/qemu-system-mips -accel qtest -monitor stdio
==12739== 
QEMU 2.12.50 monitor - type 'help' for more information
(qemu) device_add secondary-vga
Unsupported bus. Bus doesn't have property 'acpi-pcihp-bsel' set
(qemu) dump-guest-memory /dev/null 0 4096
==12739== Invalid read of size 8
==12739==    at 0x6AFCD5: object_dynamic_cast (object.c:613)
==12739==    by 0x6AFCD5: object_resolve_abs_path (object.c:1721)
==12739==    by 0x6AFD30: object_resolve_partial_path (object.c:1745)
==12739==    by 0x6AFD92: object_resolve_partial_path (object.c:1755)
==12739==    by 0x6AFD92: object_resolve_partial_path (object.c:1755)
==12739==    by 0x6AFD92: object_resolve_partial_path (object.c:1755)
==12739==    by 0x6AFE61: object_resolve_path_type (object.c:1784)
==12739==    by 0x42992F: vmcoreinfo_find (vmcoreinfo.h:41)
==12739==    by 0x42992F: dump_init (dump.c:1643)
==12739==    by 0x42992F: qmp_dump_guest_memory (dump.c:1998)
==12739==    by 0x50B6EC: hmp_dump_guest_memory (hmp.c:2051)
==12739==    by 0x4153AA: handle_hmp_command (monitor.c:3455)
==12739==    by 0x4166BB: monitor_command_cb (monitor.c:4347)
==12739==    by 0x7A35C7: readline_handle_byte (readline.c:393)
==12739==    by 0x4154E6: monitor_read (monitor.c:4330)
==12739==  Address 0x231faf40 is 16 bytes inside a block of size 256 free'd
==12739==    at 0x4C2ACBD: free (vg_replace_malloc.c:530)
==12739==    by 0x89F26B6: pixman_image_unref (in 
/usr/lib64/libpixman-1.so.0.34.0)
==12739==    by 0x689154: qemu_pixman_glyph_render (qemu-pixman.c:266)
==12739==    by 0x683FFE: vga_putcharxy.isra.7 (console.c:469)
==12739==    by 0x68551C: console_refresh (console.c:601)
==12739==    by 0x684841: text_console_update_cursor (console.c:2160)
==12739==    by 0x78F8C0: timerlist_run_timers (qemu-timer.c:536)
==12739==    by 0x78FBA5: qemu_clock_run_timers (qemu-timer.c:547)
==12739==    by 0x78FBA5: qemu_clock_run_all_timers (qemu-timer.c:674)
==12739==    by 0x790099: main_loop_wait (main-loop.c:503)
==12739==    by 0x4F2BF1: main_loop (vl.c:1848)
==12739==    by 0x3C6C69: main (vl.c:4600)
==12739==  Block was alloc'd at
==12739==    at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==12739==    by 0x89F267A: ??? (in /usr/lib64/libpixman-1.so.0.34.0)
==12739==    by 0x89FF2FD: pixman_image_create_solid_fill (in 
/usr/lib64/libpixman-1.so.0.34.0)
==12739==    by 0x68909A: qemu_pixman_glyph_render (qemu-pixman.c:255)
==12739==    by 0x683FFE: vga_putcharxy.isra.7 (console.c:469)
==12739==    by 0x68551C: console_refresh (console.c:601)
==12739==    by 0x684841: text_console_update_cursor (console.c:2160)
==12739==    by 0x78F8C0: timerlist_run_timers (qemu-timer.c:536)
==12739==    by 0x78FBA5: qemu_clock_run_timers (qemu-timer.c:547)
==12739==    by 0x78FBA5: qemu_clock_run_all_timers (qemu-timer.c:674)
==12739==    by 0x790099: main_loop_wait (main-loop.c:503)
==12739==    by 0x4F2BF1: main_loop (vl.c:1848)
==12739==    by 0x3C6C69: main (vl.c:4600)
...

 Thomas