[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 03/16] hw/arm/bcm2836: Fix crash with device_
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH v2 03/16] hw/arm/bcm2836: Fix crash with device_add bcm2837 on unsupported machines |
Date: |
Mon, 16 Jul 2018 09:09:13 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 13.07.2018 23:26, Eduardo Habkost wrote:
> On Fri, Jul 13, 2018 at 10:27:31AM +0200, Thomas Huth wrote:
>> When trying to "device_add bcm2837" on a machine that is not suitable for
>> this device, you can quickly crash QEMU afterwards, e.g. with "info qtree":
>>
>> echo "{'execute':'qmp_capabilities'} {'execute':'device_add', " \
>> "'arguments':{'driver':'bcm2837'}} {'execute': 'human-monitor-command', " \
>> "'arguments': {'command-line': 'info qtree'}}" | \
>> aarch64-softmmu/qemu-system-aarch64 -M integratorcp,accel=qtest -S -qmp
>> stdio
>>
>> {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2},
>> "package": "build-all"}, "capabilities": []}}
>> {"return": {}}
>> {"error": {"class": "GenericError", "desc": "Device 'bcm2837' can not be
>> hotplugged on this machine"}}
>> Segmentation fault (core dumped)
>>
>> The qdev_set_parent_bus() from instance_init adds a link to the child devices
>> which is not valid anymore after the bcm2837 instance has been destroyed.
>> Unfortunately, the child devices do not get destroyed / unlinked correctly
>> because both object_initialize() and object_property_add_child() increase
>> the reference count of the child objects by one, but only one reference
>> is dropped when the parent gets removed. So let's use the new functions
>> object_initialize_child() and sysbus_init_child_obj() instead to create
>> the objects, which will take care of creating the child objects with the
>> correct reference count of one.
>>
>> Signed-off-by: Thomas Huth <address@hidden>
>
> Reviewed-by: Eduardo Habkost <address@hidden>
>
> The usage of &error_abort in code that can be triggered from
> device-list-properties still makes me nervous, but that's a
> separate issue.
I first had similar thoughts, but I think it's a clear coding issue if
the abort triggers here (and not something that the user should normally
be able to trigger somehow), so error_abort should be ok in this case.
Thomas
- Re: [Qemu-devel] [PATCH v2 01/16] qom/object: Add a new function object_initialize_child(), (continued)
Re: [Qemu-devel] [PATCH v2 01/16] qom/object: Add a new function object_initialize_child(), Eduardo Habkost, 2018/07/13
[Qemu-devel] [PATCH v2 02/16] hw/core/sysbus: Add a function for creating and attaching an object, Thomas Huth, 2018/07/13
[Qemu-devel] [PATCH v2 03/16] hw/arm/bcm2836: Fix crash with device_add bcm2837 on unsupported machines, Thomas Huth, 2018/07/13
[Qemu-devel] [PATCH v2 04/16] hw/arm/armv7: Fix crash when introspecting the "iotkit" device, Thomas Huth, 2018/07/13
[Qemu-devel] [PATCH v2 05/16] hw/cpu/a15mpcore: Fix introspection problem with the a15mpcore_priv device, Thomas Huth, 2018/07/13
[Qemu-devel] [PATCH v2 06/16] hw/display/xlnx_dp: Move problematic code from instance_init to realize, Thomas Huth, 2018/07/13