[PATCH v5 3/7] qcow: Tolerate backing_fmt=, but warn on backing_fmt=raw

Eric Blake eblake at redhat.com
Fri Apr 3 17:58:55 UTC 2020


qcow has no space in the metadata to store a backing format, and there
are existing qcow images backed both by raw or by other formats
(usually qcow) images, reliant on probing to tell the difference.
While we don't recommend the creation of new qcow images (as qcow2 is
hands-down better), we can at least insist that if the user does
request a specific format without using -u, then it must be non-raw
(as a raw backing file that gets inadvertently edited into some other
format can form a security hole); if the user does not request a
specific format or lies when using -u, then the status quo of probing
for the backing format remains intact (although an upcoming patch will
warn when omitting a format request).  Thus, when this series is
complete, the only way to use a backing file for qcow without
triggering a warning is when using -F if the backing file is non-raw
to begin with.  Note that this is only for QemuOpts usage; there is no
change to the QAPI to allow a format through -blockdev.

Add a new iotest 290 just for qcow, to demonstrate the new warning.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 block/qcow.c               | 16 ++++++++-
 tests/qemu-iotests/290     | 72 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/290.out | 42 ++++++++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 130 insertions(+), 1 deletion(-)
 create mode 100755 tests/qemu-iotests/290
 create mode 100644 tests/qemu-iotests/290.out

diff --git a/block/qcow.c b/block/qcow.c
index 8973e4e565a1..bbe48b7db4d7 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -940,11 +940,12 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv,
 {
     BlockdevCreateOptions *create_options = NULL;
     BlockDriverState *bs = NULL;
-    QDict *qdict;
+    QDict *qdict = NULL;
     Visitor *v;
     const char *val;
     Error *local_err = NULL;
     int ret;
+    char *backing_fmt;

     static const QDictRenames opt_renames[] = {
         { BLOCK_OPT_BACKING_FILE,       "backing-file" },
@@ -953,6 +954,13 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv,
     };

     /* Parse options and convert legacy syntax */
+    backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
+    if (backing_fmt && !strcmp(backing_fmt, "raw")) {
+        error_setg(errp, "qcow cannot store backing format; an explicit "
+                   "backing format of raw is unsafe");
+        ret = -EINVAL;
+        goto fail;
+    }
     qdict = qemu_opts_to_qdict_filtered(opts, NULL, &qcow_create_opts, true);

     val = qdict_get_try_str(qdict, BLOCK_OPT_ENCRYPT);
@@ -1018,6 +1026,7 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv,

     ret = 0;
 fail:
+    g_free(backing_fmt);
     qobject_unref(qdict);
     bdrv_unref(bs);
     qapi_free_BlockdevCreateOptions(create_options);
@@ -1152,6 +1161,11 @@ static QemuOptsList qcow_create_opts = {
             .type = QEMU_OPT_STRING,
             .help = "File name of a base image"
         },
+        {
+            .name = BLOCK_OPT_BACKING_FMT,
+            .type = QEMU_OPT_STRING,
+            .help = "Format of the backing image (caution: raw backing is unsafe)",
+        },
         {
             .name = BLOCK_OPT_ENCRYPT,
             .type = QEMU_OPT_BOOL,
diff --git a/tests/qemu-iotests/290 b/tests/qemu-iotests/290
new file mode 100755
index 000000000000..8482de58cb4b
--- /dev/null
+++ b/tests/qemu-iotests/290
@@ -0,0 +1,72 @@
+#!/usr/bin/env bash
+#
+# Test qcow backing file warnings
+#
+# Copyright (C) 2020 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1	# failure is the default!
+
+_cleanup()
+{
+	_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow
+_supported_proto file
+_supported_os Linux
+
+size=128M
+
+echo
+echo "== qcow backed by qcow =="
+
+TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+_img_info
+_make_test_img -b "$TEST_IMG.base" -F $IMGFMT
+_img_info
+
+echo
+echo "== mismatched command line detection =="
+
+_make_test_img -b "$TEST_IMG.base" -F vmdk
+# Use of -u bypasses the backing format sanity check
+_make_test_img -u -b "$TEST_IMG.base" -F vmdk $size
+_img_info
+
+echo
+echo "== qcow backed by raw =="
+
+rm "$TEST_IMG.base"
+truncate --size=$size "$TEST_IMG.base"
+_make_test_img -b "$TEST_IMG.base"
+_img_info
+_make_test_img -b "$TEST_IMG.base" -F raw
+_img_info
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/290.out b/tests/qemu-iotests/290.out
new file mode 100644
index 000000000000..5b4ae3196d04
--- /dev/null
+++ b/tests/qemu-iotests/290.out
@@ -0,0 +1,42 @@
+QA output created by 290
+
+== qcow backed by qcow ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 128 MiB (134217728 bytes)
+cluster_size: 512
+backing file: TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 128 MiB (134217728 bytes)
+cluster_size: 512
+backing file: TEST_DIR/t.IMGFMT.base
+
+== mismatched command line detection ==
+qemu-img: TEST_DIR/t.IMGFMT: invalid VMDK image descriptor
+Could not open backing image to determine size.
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=vmdk
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 128 MiB (134217728 bytes)
+cluster_size: 512
+backing file: TEST_DIR/t.IMGFMT.base
+
+== qcow backed by raw ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 128 MiB (134217728 bytes)
+cluster_size: 512
+backing file: TEST_DIR/t.IMGFMT.base
+qemu-img: TEST_DIR/t.IMGFMT: IMGFMT cannot store backing format; an explicit backing format of raw is unsafe
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 128 MiB (134217728 bytes)
+cluster_size: 512
+backing file: TEST_DIR/t.IMGFMT.base
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 79c6dfc85d1b..c4dc86a9cf73 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -296,3 +296,4 @@
 286 rw quick
 288 quick
 289 rw quick
+290 backing quick
-- 
2.26.0.rc2




More information about the libvir-list mailing list