qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] block/mirror: fix core when using iothreads


From: Peng Liang
Subject: [PATCH] block/mirror: fix core when using iothreads
Date: Wed, 26 Aug 2020 21:19:10 +0800

We found an issue when doing block-commit with iothreads, which tries to
dereference a NULL pointer.

<main thread>                      |<IO thread>
                                   |
mirror_start_job                   |
 1. bdrv_ref(mirror_top_bs);       |
    bdrv_drained_begin(bs);        |
    bdrv_append(mirror_top_bs,     |
                bs, &local_err);   |
    bdrv_drained_end(bs);          |
                                   |
 2. s = block_job_create(...);     |
                                   |bdrv_mirror_top_pwritev
                                   | MirrorBDSOpaque *s = bs->opaque;
                                   | bool copy_to_target;
                                   | copy_to_target = s->job->ret >= 0 &&
                                   |     s->job->copy_mode ==
                                   |     MIRROR_COPY_MODE_WRITE_BLOCKING;
                                   | (s->job is not NULL until 3!)
 3. bs_opaque->job = s;            |

Just moving step 2 & 3 before 1 can avoid this.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
---
 block/mirror.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index e8e8844afc40..7c872be71149 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1600,6 +1600,16 @@ static BlockJob *mirror_start_job(
     mirror_top_bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
                                           BDRV_REQ_NO_FALLBACK;
     bs_opaque = g_new0(MirrorBDSOpaque, 1);
+    /* Make sure that the source is not resized while the job is running */
+    s = block_job_create(job_id, driver, NULL, bs,
+                         BLK_PERM_CONSISTENT_READ,
+                         BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
+                         BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD, speed,
+                         creation_flags, cb, opaque, errp);
+    if (!s) {
+        goto fail;
+    }
+    bs_opaque->job = s;
     mirror_top_bs->opaque = bs_opaque;
 
     /* bdrv_append takes ownership of the mirror_top_bs reference, need to keep
@@ -1612,19 +1622,8 @@ static BlockJob *mirror_start_job(
     if (local_err) {
         bdrv_unref(mirror_top_bs);
         error_propagate(errp, local_err);
-        return NULL;
-    }
-
-    /* Make sure that the source is not resized while the job is running */
-    s = block_job_create(job_id, driver, NULL, mirror_top_bs,
-                         BLK_PERM_CONSISTENT_READ,
-                         BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
-                         BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD, speed,
-                         creation_flags, cb, opaque, errp);
-    if (!s) {
         goto fail;
     }
-    bs_opaque->job = s;
 
     /* The block job now has a reference to this node */
     bdrv_unref(mirror_top_bs);
-- 
2.18.4




reply via email to

[Prev in Thread] Current Thread [Next in Thread]