From bf4c15a3debbe68b6eb25c52174843470a9c014f Mon Sep 17 00:00:00 2001

From: Stefan Hajnoczi <stefanha@redhat.com>

Date: Tue, 11 Jan 2022 15:36:12 +0000

Subject: [PATCH 3/6] block-backend: prevent dangling BDS pointers across

 aio_poll()

RH-Author: Stefan Hajnoczi <stefanha@redhat.com>

RH-MergeRequest: 109: block-backend: prevent dangling BDS pointers across aio_poll()

RH-Commit: [1/2] da5a59eddff0dc10be7de8e291fa675143d11d73

RH-Bugzilla: 2021778 2036178

RH-Acked-by: Hanna Reitz <hreitz@redhat.com>

RH-Acked-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

RH-Acked-by: Kevin Wolf <kwolf@redhat.com>

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>

(cherry picked from commit 1e3552dbd28359d35967b7c28dc86cde1bc29205)

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

 block/block-backend.c | 19 +++++++++++++++++--

 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c

index 12ef80ea17..23e727199b 100644

--- a/block/block-backend.c

+++ b/block/block-backend.c

@@ -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);

     }

 }

-- 

2.27.0