[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] block: Avoid stale pointer dereference in blk_get_aio_contex
From: |
Kevin Wolf |
Subject: |
Re: [PATCH] block: Avoid stale pointer dereference in blk_get_aio_context() |
Date: |
Fri, 10 Jul 2020 10:53:16 +0200 |
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