Re: [PATCH] block: Avoid stale pointer dereference in blk_get_aio_context()

[Kevin Wolf]

Am 09.07.2020 um 15:50 hat Greg Kurz geschrieben:
> 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>

Thanks, applied to the block branch.

Kevin