OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

block: Avoid stale pointer dereference in blk_get_aio_context()

Browse Source 
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>
This commit is contained in:
Greg Kurz 2020-07-09 15:50:45 +02:00 committed by Kevin Wolf
parent d9f059aa6c
commit e6cada9231
1 changed files with 3 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
block/block-backend.c
View File

@ -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);
}
/*