block-backend: prevent dangling BDS pointers across aio_poll()
The BlockBackend root child can change when aio_poll() is invoked. This happens when a temporary filter node is removed upon blockjob completion, for example. Functions in block/block-backend.c must be aware of this when using a blk_bs() pointer across aio_poll() because the BlockDriverState refcnt may reach 0, resulting in a stale pointer. One example is scsi_device_purge_requests(), which calls blk_drain() to wait for in-flight requests to cancel. If the backup blockjob is active, then the BlockBackend root child is a temporary filter BDS owned by the blockjob. The blockjob can complete during bdrv_drained_begin() and the last reference to the BDS is released when the temporary filter node is removed. This results in a use-after-free when blk_drain() calls bdrv_drained_end(bs) on the dangling pointer. Explicitly hold a reference to bs across block APIs that invoke aio_poll(). Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2021778 Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2036178 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20220111153613.25453-2-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
parent
bb01ea7311
commit
1e3552dbd2
@ -822,16 +822,22 @@ BlockBackend *blk_by_public(BlockBackendPublic *public)
|
||||
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) {
|
||||
bs = blk_bs(blk);
|
||||
BlockDriverState *bs = blk_bs(blk);
|
||||
|
||||
/*
|
||||
* Take a ref in case blk_bs() changes across bdrv_drained_begin(), for
|
||||
* example, if a temporary filter node is removed by a blockjob.
|
||||
*/
|
||||
bdrv_ref(bs);
|
||||
bdrv_drained_begin(bs);
|
||||
throttle_group_detach_aio_context(tgm);
|
||||
throttle_group_attach_aio_context(tgm, qemu_get_aio_context());
|
||||
bdrv_drained_end(bs);
|
||||
bdrv_unref(bs);
|
||||
}
|
||||
|
||||
blk_update_root_state(blk);
|
||||
@ -1705,6 +1711,7 @@ void blk_drain(BlockBackend *blk)
|
||||
BlockDriverState *bs = blk_bs(blk);
|
||||
|
||||
if (bs) {
|
||||
bdrv_ref(bs);
|
||||
bdrv_drained_begin(bs);
|
||||
}
|
||||
|
||||
@ -1714,6 +1721,7 @@ void blk_drain(BlockBackend *blk)
|
||||
|
||||
if (bs) {
|
||||
bdrv_drained_end(bs);
|
||||
bdrv_unref(bs);
|
||||
}
|
||||
}
|
||||
|
||||
@ -2044,10 +2052,13 @@ static int blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context,
|
||||
int ret;
|
||||
|
||||
if (bs) {
|
||||
bdrv_ref(bs);
|
||||
|
||||
if (update_root_node) {
|
||||
ret = bdrv_child_try_set_aio_context(bs, new_context, blk->root,
|
||||
errp);
|
||||
if (ret < 0) {
|
||||
bdrv_unref(bs);
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
@ -2057,6 +2068,8 @@ static int blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context,
|
||||
throttle_group_attach_aio_context(tgm, new_context);
|
||||
bdrv_drained_end(bs);
|
||||
}
|
||||
|
||||
bdrv_unref(bs);
|
||||
}
|
||||
|
||||
blk->ctx = new_context;
|
||||
@ -2326,11 +2339,13 @@ void blk_io_limits_disable(BlockBackend *blk)
|
||||
ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
|
||||
assert(tgm->throttle_state);
|
||||
if (bs) {
|
||||
bdrv_ref(bs);
|
||||
bdrv_drained_begin(bs);
|
||||
}
|
||||
throttle_group_unregister_tgm(tgm);
|
||||
if (bs) {
|
||||
bdrv_drained_end(bs);
|
||||
bdrv_unref(bs);
|
||||
}
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user