Paolo Bonzini <pbonz...@redhat.com> writes:
> 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-sta...@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
This broke tests/qemu-iotests/067. First of four similar diff hunks:
--- tests/qemu-iotests/067.out 2014-06-06 13:51:21.231106345 +0200
+++ tests/qemu-iotests/067.out.bad 2014-06-26 13:11:36.826825145 +0200
@@ -9,7 +9,6 @@
{"return": [{"io-status": "ok", "device": "disk", "locked": false,
"removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image":
{"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size":
65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type":
"qcow2", "data": {"compat": "1.1", "lazy-refcounts": false}}, "dirty-flag":
false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2",
"iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file":
"TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"},
{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true,
"tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false,
"removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0",
"locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
{"return": {}}
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"DEVICE_DELETED", "data": {"path":
"/machine/peripheral/virtio0/virtio-backend"}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"DEVICE_DELETED", "data": {"device": "virtio0", "path":
"/machine/peripheral/virtio0"}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"RESET"}
{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false,
"removable": true, "tray_open": false, "type": "unknown"}, {"device":
"floppy0", "locked": false, "removable": true, "tray_open": false, "type":
"unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open":
false, "type": "unknown"}]}
The '{"return": {}}' immediately before the deleted line is the reply to
QMP command '{ "execute": "device_del", "arguments": { "id": "virtio0" }
}', which deletes a virtio-blk-pci device.
Before the patch, we get a DEVICE_DELETED event both for the
virtio-blk-pci device (second event) and its virtio-blk-device child
(first event). The patch suppresses the event for the child.
Intentional?
If yes, I'll post the obvious patch to the expected test output.
