Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.10-rc4?] acpi: pcihp: fix use-after-free for machines previous pc-1.7 compat

[Thomas Huth]

On 23.08.2017 07:40, Thomas Huth wrote:
> On 23.08.2017 02:10, Philippe Mathieu-Daudé wrote:
>> On 08/22/2017 07:42 PM, Michael S. Tsirkin wrote:
>>> On Tue, Aug 22, 2017 at 06:43:43PM -0300, Philippe Mathieu-Daudé wrote:
>>>> 9e047b982452 "piix4: add acpi pci hotplug support" introduced a new
>>>> property
>>>> 'use_acpi_pci_hotplug' for pc-1.7 and older machines.
>>>> c24d5e0b91d1 "convert ACPI PCI hotplug to use hotplug-handler API"
>>>> added the
>>>> qbus hotplug handlers but forgot to check for the 'use_acpi_pci_hotplug'
>>>> property.
>>>>
>>>> Check for use_acpi_pci_hotplug before calling
>>>> acpi_pcihp_device_[un]plug_cb().
> [...]
>>>> Reported-by: Thomas Huth <address@hidden>
>>>> Message-Id: <address@hidden>
>>>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>>>
>>> Looks like this is a very old bug, isn't it?
>>> Objections to merging this after the release?
>>
>> Yes, I'm also inclined to delay it so we can release 2.10, I tagged
>> "2.10-rc4" since Thomas sent it as a bug within the 2.10 window so I'll
>> let him decide if it is worth crying wolf :) It's very likely no-one but
>> him used pre-pc-i440fx-1.7 the last 3 years, not even thinking about hot
>> plugging AHCI devices :D
> 
> I'm fine if this gets included in 2.11 - it's quite unlikely that a user
> tries hot-plug ahci on such an old machine type, I think. But we maybe
> should include this in the 2.10.1 stable release, so I'm putting
> qemu-stable on CC now.
> 
> Anyway, your patch seems to fix the issue for me, thanks!
> 
> Tested-by: Thomas Huth <address@hidden>

... No, I was too fast here. I'm afraid, it still crashes with mips64el:

$ valgrind mips64el-softmmu/qemu-system-mips64el -S -nographic -M 
malta,accel=qtest
==17935== Memcheck, a memory error detector
==17935== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==17935== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==17935== Command: mips64el-softmmu/qemu-system-mips64el -S -nographic -M 
malta,accel=qtest
==17935== 
QEMU 2.9.93 monitor - type 'help' for more information
(qemu) device_add ich9-ahci
==17935== Invalid read of size 8
==17935==    at 0x5F6F10: object_unparent (object.c:445)
==17935==    by 0x4BB2C8: device_unparent (qdev.c:1095)
==17935==    by 0x5F77C4: object_finalize_child_property (object.c:1396)
==17935==    by 0x5F6706: object_property_del_child.isra.7 (object.c:427)
==17935==    by 0x448BC8: qdev_device_add (qdev-monitor.c:634)
==17935==    by 0x449122: qmp_device_add (qdev-monitor.c:807)
==17935==    by 0x462B29: hmp_device_add (hmp.c:1925)
==17935==    by 0x370F83: handle_hmp_command (monitor.c:3119)
==17935==    by 0x371E59: monitor_command_cb (monitor.c:3922)
==17935==    by 0x6D3187: readline_handle_byte (readline.c:393)
==17935==    by 0x371211: monitor_read (monitor.c:3905)
==17935==    by 0x6699D3: mux_chr_read (char-mux.c:216)
==17935==  Address 0x21c549d8 is 30,328 bytes inside a block of size 36,288 
free'd
==17935==    at 0x4C2ACDD: free (vg_replace_malloc.c:530)
==17935==    by 0xA04EBCD: g_free (in /usr/lib64/libglib-2.0.so.0.5000.3)
==17935==    by 0x4FB94E: pci_ich9_uninit (ich.c:161)
==17935==    by 0x5350FB: pci_qdev_unrealize (pci.c:1083)
==17935==    by 0x4BCD39: device_set_realized (qdev.c:988)
==17935==    by 0x5F622D: property_set_bool (object.c:1886)
==17935==    by 0x5FA31E: object_property_set_qobject (qom-qobject.c:27)
==17935==    by 0x5F7FCF: object_property_set_bool (object.c:1162)
==17935==    by 0x448B93: qdev_device_add (qdev-monitor.c:630)
==17935==    by 0x449122: qmp_device_add (qdev-monitor.c:807)
==17935==    by 0x462B29: hmp_device_add (hmp.c:1925)
==17935==    by 0x370F83: handle_hmp_command (monitor.c:3119)
==17935==  Block was alloc'd at
==17935==    at 0x4C2B975: calloc (vg_replace_malloc.c:711)
==17935==    by 0xA04EB15: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5000.3)
==17935==    by 0x4FB28F: ahci_realize (ahci.c:1468)
==17935==    by 0x4FB9D8: pci_ich9_ahci_realize (ich.c:115)
==17935==    by 0x5366BD: pci_qdev_realize (pci.c:2002)
==17935==    by 0x4BCCB9: device_set_realized (qdev.c:914)
==17935==    by 0x5F622D: property_set_bool (object.c:1886)
==17935==    by 0x5FA31E: object_property_set_qobject (qom-qobject.c:27)
==17935==    by 0x5F7FCF: object_property_set_bool (object.c:1162)
==17935==    by 0x448B93: qdev_device_add (qdev-monitor.c:630)
==17935==    by 0x449122: qmp_device_add (qdev-monitor.c:807)
==17935==    by 0x462B29: hmp_device_add (hmp.c:1925)

Do you've got an idea how to fix that, too?

 Thomas