cryptospore / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone
495e37
From 87f3b10dc600ac12272ee6cdc67571910ea722f6 Mon Sep 17 00:00:00 2001
495e37
From: Stefan Hajnoczi <stefanha@redhat.com>
495e37
Date: Tue, 11 Jan 2022 15:36:12 +0000
495e37
Subject: [PATCH 10/12] block-backend: prevent dangling BDS pointers across
495e37
 aio_poll()
495e37
495e37
RH-Author: Hanna Reitz <hreitz@redhat.com>
495e37
RH-MergeRequest: 71: block-backend: prevent dangling BDS pointers across aio_poll()
495e37
RH-Commit: [1/2] 1b4cab39bf8c933ab910293a29bfceaa9e821068 (hreitz/qemu-kvm-c-9-s)
495e37
RH-Bugzilla: 2040123
495e37
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
495e37
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
495e37
RH-Acked-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
495e37
495e37
The BlockBackend root child can change when aio_poll() is invoked. This
495e37
happens when a temporary filter node is removed upon blockjob
495e37
completion, for example.
495e37
495e37
Functions in block/block-backend.c must be aware of this when using a
495e37
blk_bs() pointer across aio_poll() because the BlockDriverState refcnt
495e37
may reach 0, resulting in a stale pointer.
495e37
495e37
One example is scsi_device_purge_requests(), which calls blk_drain() to
495e37
wait for in-flight requests to cancel. If the backup blockjob is active,
495e37
then the BlockBackend root child is a temporary filter BDS owned by the
495e37
blockjob. The blockjob can complete during bdrv_drained_begin() and the
495e37
last reference to the BDS is released when the temporary filter node is
495e37
removed. This results in a use-after-free when blk_drain() calls
495e37
bdrv_drained_end(bs) on the dangling pointer.
495e37
495e37
Explicitly hold a reference to bs across block APIs that invoke
495e37
aio_poll().
495e37
495e37
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2021778
495e37
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2036178
495e37
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
495e37
Message-Id: <20220111153613.25453-2-stefanha@redhat.com>
495e37
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
495e37
(cherry picked from commit 1e3552dbd28359d35967b7c28dc86cde1bc29205)
495e37
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
495e37
---
495e37
 block/block-backend.c | 19 +++++++++++++++++--
495e37
 1 file changed, 17 insertions(+), 2 deletions(-)
495e37
495e37
diff --git a/block/block-backend.c b/block/block-backend.c
495e37
index 12ef80ea17..23e727199b 100644
495e37
--- a/block/block-backend.c
495e37
+++ b/block/block-backend.c
495e37
@@ -822,16 +822,22 @@ BlockBackend *blk_by_public(BlockBackendPublic *public)
495e37
 void blk_remove_bs(BlockBackend *blk)
495e37
 {
495e37
     ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
495e37
-    BlockDriverState *bs;
495e37
     BdrvChild *root;
495e37
 
495e37
     notifier_list_notify(&blk->remove_bs_notifiers, blk);
495e37
     if (tgm->throttle_state) {
495e37
-        bs = blk_bs(blk);
495e37
+        BlockDriverState *bs = blk_bs(blk);
495e37
+
495e37
+        /*
495e37
+         * Take a ref in case blk_bs() changes across bdrv_drained_begin(), for
495e37
+         * example, if a temporary filter node is removed by a blockjob.
495e37
+         */
495e37
+        bdrv_ref(bs);
495e37
         bdrv_drained_begin(bs);
495e37
         throttle_group_detach_aio_context(tgm);
495e37
         throttle_group_attach_aio_context(tgm, qemu_get_aio_context());
495e37
         bdrv_drained_end(bs);
495e37
+        bdrv_unref(bs);
495e37
     }
495e37
 
495e37
     blk_update_root_state(blk);
495e37
@@ -1705,6 +1711,7 @@ void blk_drain(BlockBackend *blk)
495e37
     BlockDriverState *bs = blk_bs(blk);
495e37
 
495e37
     if (bs) {
495e37
+        bdrv_ref(bs);
495e37
         bdrv_drained_begin(bs);
495e37
     }
495e37
 
495e37
@@ -1714,6 +1721,7 @@ void blk_drain(BlockBackend *blk)
495e37
 
495e37
     if (bs) {
495e37
         bdrv_drained_end(bs);
495e37
+        bdrv_unref(bs);
495e37
     }
495e37
 }
495e37
 
495e37
@@ -2044,10 +2052,13 @@ static int blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context,
495e37
     int ret;
495e37
 
495e37
     if (bs) {
495e37
+        bdrv_ref(bs);
495e37
+
495e37
         if (update_root_node) {
495e37
             ret = bdrv_child_try_set_aio_context(bs, new_context, blk->root,
495e37
                                                  errp);
495e37
             if (ret < 0) {
495e37
+                bdrv_unref(bs);
495e37
                 return ret;
495e37
             }
495e37
         }
495e37
@@ -2057,6 +2068,8 @@ static int blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context,
495e37
             throttle_group_attach_aio_context(tgm, new_context);
495e37
             bdrv_drained_end(bs);
495e37
         }
495e37
+
495e37
+        bdrv_unref(bs);
495e37
     }
495e37
 
495e37
     blk->ctx = new_context;
495e37
@@ -2326,11 +2339,13 @@ void blk_io_limits_disable(BlockBackend *blk)
495e37
     ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
495e37
     assert(tgm->throttle_state);
495e37
     if (bs) {
495e37
+        bdrv_ref(bs);
495e37
         bdrv_drained_begin(bs);
495e37
     }
495e37
     throttle_group_unregister_tgm(tgm);
495e37
     if (bs) {
495e37
         bdrv_drained_end(bs);
495e37
+        bdrv_unref(bs);
495e37
     }
495e37
 }
495e37
 
495e37
-- 
495e37
2.27.0
495e37