[Qemu-devel] [PATCH 1/2] commit: Fix use after free in completion

Kevin Wolf posted 2 patches 8 years, 5 months ago
[Qemu-devel] [PATCH 1/2] commit: Fix use after free in completion
Posted by Kevin Wolf 8 years, 5 months ago
The final bdrv_set_backing_hd() could be working on already freed nodes
because the commit job drops its references (through BlockBackends) to
both overlay_bs and top already a bit earlier.

One way to trigger the bug is hot unplugging a disk for which
blockdev_mark_auto_del() cancels the block job.

Fix this by taking BDS-level references while we're still using the
nodes.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/commit.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/block/commit.c b/block/commit.c
index a3028b2..af6fa68 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -89,6 +89,10 @@ static void commit_complete(BlockJob *job, void *opaque)
     int ret = data->ret;
     bool remove_commit_top_bs = false;
 
+    /* Make sure overlay_bs and top stay around until bdrv_set_backing_hd() */
+    bdrv_ref(top);
+    bdrv_ref(overlay_bs);
+
     /* Remove base node parent that still uses BLK_PERM_WRITE/RESIZE before
      * the normal backing chain can be restored. */
     blk_unref(s->base);
@@ -124,6 +128,9 @@ static void commit_complete(BlockJob *job, void *opaque)
     if (remove_commit_top_bs) {
         bdrv_set_backing_hd(overlay_bs, top, &error_abort);
     }
+
+    bdrv_unref(overlay_bs);
+    bdrv_unref(top);
 }
 
 static void coroutine_fn commit_run(void *opaque)
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH 1/2] commit: Fix use after free in completion
Posted by Kevin Wolf 8 years, 4 months ago
Am 02.06.2017 um 23:12 hat Kevin Wolf geschrieben:
> The final bdrv_set_backing_hd() could be working on already freed nodes
> because the commit job drops its references (through BlockBackends) to
> both overlay_bs and top already a bit earlier.
> 
> One way to trigger the bug is hot unplugging a disk for which
> blockdev_mark_auto_del() cancels the block job.
> 
> Fix this by taking BDS-level references while we're still using the
> nodes.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Cc: qemu-stable@nongnu.org