From 1bf26076d677f693dd99a6e8ef2eca69b842d873 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Tue, 9 Mar 2021 18:34:51 +0100
Subject: [PATCH] stream: Don't crash when node permission is denied

The image streaming block job restricts shared permissions of the nodes
it accesses. This can obviously fail when other users already got these
permissions. &error_abort is therefore wrong and can crash. Handle these
errors gracefully and just fail starting the block job.

Reported-by: Nini Gu <ngu@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210309173451.45152-1-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/stream.c                        | 15 +++++++++++----
 tests/qemu-iotests/tests/qsd-jobs     | 20 ++++++++++++++++++++
 tests/qemu-iotests/tests/qsd-jobs.out | 10 ++++++++++
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/block/stream.c b/block/stream.c
index 1fa742b0db..97bee482dc 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -206,7 +206,7 @@ void stream_start(const char *job_id, BlockDriverState *bs,
                   const char *filter_node_name,
                   Error **errp)
 {
-    StreamBlockJob *s;
+    StreamBlockJob *s = NULL;
     BlockDriverState *iter;
     bool bs_read_only;
     int basic_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
@@ -214,6 +214,7 @@ void stream_start(const char *job_id, BlockDriverState *bs,
     BlockDriverState *cor_filter_bs = NULL;
     BlockDriverState *above_base;
     QDict *opts;
+    int ret;
 
     assert(!(base && bottom));
     assert(!(backing_file_str && bottom));
@@ -303,7 +304,7 @@ void stream_start(const char *job_id, BlockDriverState *bs,
      * queried only at the job start and then cached.
      */
     if (block_job_add_bdrv(&s->common, "active node", bs, 0,
-                           basic_flags | BLK_PERM_WRITE, &error_abort)) {
+                           basic_flags | BLK_PERM_WRITE, errp)) {
         goto fail;
     }
 
@@ -320,8 +321,11 @@ void stream_start(const char *job_id, BlockDriverState *bs,
     for (iter = bdrv_filter_or_cow_bs(bs); iter != base;
          iter = bdrv_filter_or_cow_bs(iter))
     {
-        block_job_add_bdrv(&s->common, "intermediate node", iter, 0,
-                           basic_flags, &error_abort);
+        ret = block_job_add_bdrv(&s->common, "intermediate node", iter, 0,
+                                 basic_flags, errp);
+        if (ret < 0) {
+            goto fail;
+        }
     }
 
     s->base_overlay = base_overlay;
@@ -337,6 +341,9 @@ void stream_start(const char *job_id, BlockDriverState *bs,
     return;
 
 fail:
+    if (s) {
+        job_early_fail(&s->common.job);
+    }
     if (cor_filter_bs) {
         bdrv_cor_filter_drop(cor_filter_bs);
     }
diff --git a/tests/qemu-iotests/tests/qsd-jobs b/tests/qemu-iotests/tests/qsd-jobs
index 1a1c534fac..972b6b3898 100755
--- a/tests/qemu-iotests/tests/qsd-jobs
+++ b/tests/qemu-iotests/tests/qsd-jobs
@@ -30,6 +30,7 @@ status=1	# failure is the default!
 _cleanup()
 {
     _cleanup_test_img
+    rm -f "$SOCK_DIR/nbd.sock"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -59,6 +60,25 @@ $QSD --chardev stdio,id=stdio --monitor chardev=stdio \
 {"execute": "quit"}
 EOF
 
+echo
+echo "=== Streaming can't get permission on base node ==="
+echo
+
+# Just make sure that this doesn't crash
+$QSD --chardev stdio,id=stdio --monitor chardev=stdio \
+    --blockdev node-name=file_base,driver=file,filename="$TEST_IMG.base" \
+    --blockdev node-name=fmt_base,driver=qcow2,file=file_base \
+    --blockdev node-name=file_overlay,driver=file,filename="$TEST_IMG" \
+    --blockdev node-name=fmt_overlay,driver=qcow2,file=file_overlay,backing=fmt_base \
+    --nbd-server addr.type=unix,addr.path="$SOCK_DIR/nbd.sock" \
+    --export type=nbd,id=export1,node-name=fmt_base,writable=on,name=export1 \
+    <<EOF | _filter_qmp
+{"execute": "qmp_capabilities"}
+{"execute": "block-stream",
+  "arguments": {"device": "fmt_overlay", "job-id": "job0"}}
+{"execute": "quit"}
+EOF
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/tests/qsd-jobs.out b/tests/qemu-iotests/tests/qsd-jobs.out
index e3a684b34d..05e1165e80 100644
--- a/tests/qemu-iotests/tests/qsd-jobs.out
+++ b/tests/qemu-iotests/tests/qsd-jobs.out
@@ -19,4 +19,14 @@ QMP_VERSION
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "job0"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "job0"}}
+
+=== Streaming can't get permission on base node ===
+
+QMP_VERSION
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "job0"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "job0"}}
+{"error": {"class": "GenericError", "desc": "Conflicts with use by a block device as 'root', which uses 'write' on fmt_base"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "export1"}}
 *** done