aboutsummaryrefslogtreecommitdiff
path: root/block/block-backend.c
diff options
context:
space:
mode:
authorGreg Kurz <groug@kaod.org>2020-07-09 15:50:45 +0200
committerKevin Wolf <kwolf@redhat.com>2020-07-14 15:24:15 +0200
commite6cada9231af022ffc2e351c70dfaea8530496e1 (patch)
tree3773d5b1da5ad61af2558fdad1982e859e583062 /block/block-backend.c
parentd9f059aa6cfccefaffa3532556e966df4a99ece2 (diff)
block: Avoid stale pointer dereference in blk_get_aio_context()
It is possible for blk_remove_bs() to race with blk_drain_all(), causing the latter to dereference a stale blk->root pointer: blk_remove_bs(blk) bdrv_root_unref_child(blk->root) child_bs = blk->root->bs bdrv_detach_child(blk->root) ... g_free(blk->root) <============== blk->root becomes stale bdrv_unref(child_bs) <============ yield at some point A blk_drain_all() can be triggered by some guest action in the meantime, eg. on POWER, SLOF might disable bus mastering on a virtio-scsi-pci device: virtio_write_config() virtio_pci_stop_ioeventfd() virtio_bus_stop_ioeventfd() virtio_scsi_dataplane_stop() blk_drain_all() blk_get_aio_context() bs = blk->root ? blk->root->bs : NULL ^^^^^^^^^ stale Then, depending on one's luck, QEMU either crashes with SEGV or hits the assertion in blk_get_aio_context(). blk->root is set by blk_insert_bs() which calls bdrv_root_attach_child() first. The blk_remove_bs() function should rollback the changes made by blk_insert_bs() in the opposite order (or it should be documented somewhere why this isn't the case). Clear blk->root before calling bdrv_root_unref_child() in blk_remove_bs(). Signed-off-by: Greg Kurz <groug@kaod.org> Message-Id: <159430264541.389456.11925072456012783045.stgit@bahia.lan> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Diffstat (limited to 'block/block-backend.c')
-rw-r--r--block/block-backend.c4
1 files changed, 3 insertions, 1 deletions
diff --git a/block/block-backend.c b/block/block-backend.c
index 6936b25c83..0bf0188133 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -808,6 +808,7 @@ void blk_remove_bs(BlockBackend *blk)
{
ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
BlockDriverState *bs;
+ BdrvChild *root;
notifier_list_notify(&blk->remove_bs_notifiers, blk);
if (tgm->throttle_state) {
@@ -825,8 +826,9 @@ void blk_remove_bs(BlockBackend *blk)
* to avoid that and a potential QEMU crash.
*/
blk_drain(blk);
- bdrv_root_unref_child(blk->root);
+ root = blk->root;
blk->root = NULL;
+ bdrv_root_unref_child(root);
}
/*