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

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(a)redhat.com&gt;

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