diff options
author | Paolo Bonzini | 2014-06-11 14:52:09 +0200 |
---|---|---|
committer | Michael S. Tsirkin | 2014-06-19 17:44:21 +0200 |
commit | 5942a19040fed313b316ab7b6e3d2d8e7b1625bb (patch) | |
tree | 2ddce7a63358ccbe45abd86d28509b0d16370b9e /hw/core | |
parent | qdev: reorganize error reporting in bus_set_realized (diff) | |
download | qemu-5942a19040fed313b316ab7b6e3d2d8e7b1625bb.tar.gz qemu-5942a19040fed313b316ab7b6e3d2d8e7b1625bb.tar.xz qemu-5942a19040fed313b316ab7b6e3d2d8e7b1625bb.zip |
qdev: recursively unrealize devices when unrealizing bus
When the patch was posted that became 5c21ce7 (qdev: Realize buses
on device realization, 2014-03-12), it included recursive realization
and unrealization of devices when the bus's "realized" property
was toggled.
However, due to the same old worries about recursive realization
and prerequisites not being realized yet, those hunks were dropped when
committing the patch. Unfortunately, this causes a use-after-free bug
(easily reproduced by a PCI hot-unplug action).
Before the patch, device_unparent behaved as follows:
for each child bus
unparent bus ----------------------------.
| for each child device |
| unparent device ---------------. |
| | unrealize device | |
| | call dc->unparent | |
| '------------------------------- |
'----------------------------------------'
unrealize device
After the patch, it behaves as follows instead:
unrealize device --------------------.
| for each child bus |
| unrealize bus (A) |
'------------------------------------'
for each child bus
unparent bus ----------------------.
| for each child device |
| unrealize device (B) |
| call dc->unparent |
'----------------------------------'
At the step marked (B) the device might use data from the bus that is
not available anymore due to step (A).
To fix this, we need to unrealize devices before step (A). To sidestep
concerns about recursive realization, only do recursive unrealization
and leave the "value && !bus->realized" case as it is.
The resulting flow is:
for each child bus
unrealize bus ---------------------.
| for each child device |
| unrealize device (B) |
| call bc->unrealize (A) |
'----------------------------------'
unrealize device
for each child bus
unparent bus ----------------------.
| for each child device |
| unparent device |
'----------------------------------'
where everything is "powered down" before it is unassembled.
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Diffstat (limited to 'hw/core')
-rw-r--r-- | hw/core/qdev.c | 13 |
1 files changed, 12 insertions, 1 deletions
diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 65aa041285..b9cd4fc814 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -568,14 +568,25 @@ static void bus_set_realized(Object *obj, bool value, Error **errp) { BusState *bus = BUS(obj); BusClass *bc = BUS_GET_CLASS(bus); + BusChild *kid; Error *local_err = NULL; if (value && !bus->realized) { if (bc->realize) { bc->realize(bus, &local_err); } + + /* TODO: recursive realization */ } else if (!value && bus->realized) { - if (bc->unrealize) { + QTAILQ_FOREACH(kid, &bus->children, sibling) { + DeviceState *dev = kid->child; + object_property_set_bool(OBJECT(dev), false, "realized", + &local_err); + if (local_err != NULL) { + break; + } + } + if (bc->unrealize && local_err == NULL) { bc->unrealize(bus, &local_err); } } |