From: "Jose R. Ziviani" Date: Thu, 2 Sep 2021 16:13:08 -0300 Subject: Revert "qemu-img: Require -F with -b backing image" This reverts commit 497a30dbb065937d67f6c43af6dd78492e1d6f6d. References: bsc#1190135 Signed-off-by: Jose R Ziviani --- block.c | 37 +++++++++++++++++++++++---------- docs/about/deprecated.rst | 23 ++++++++++++++++++++ docs/about/removed-features.rst | 19 ----------------- qemu-img.c | 6 ++---- tests/qemu-iotests/040 | 4 ++-- tests/qemu-iotests/041 | 6 ++---- tests/qemu-iotests/114 | 18 ++++++++-------- tests/qemu-iotests/114.out | 11 ++++++---- tests/qemu-iotests/301 | 4 +++- tests/qemu-iotests/301.out | 16 ++++++++++++-- 10 files changed, 88 insertions(+), 56 deletions(-) diff --git a/block.c b/block.c index 0ac5b163d2aa19368ff54f2bc04a..a4dda8c7b6e1c76e7e5c8712475b 100644 --- a/block.c +++ b/block.c @@ -5337,7 +5337,7 @@ int coroutine_fn bdrv_co_check(BlockDriverState *bs, * -ENOTSUP - format driver doesn't support changing the backing file */ int bdrv_change_backing_file(BlockDriverState *bs, const char *backing_file, - const char *backing_fmt, bool require) + const char *backing_fmt, bool warn) { BlockDriver *drv = bs->drv; int ret; @@ -5351,8 +5351,10 @@ int bdrv_change_backing_file(BlockDriverState *bs, const char *backing_file, return -EINVAL; } - if (require && backing_file && !backing_fmt) { - return -EINVAL; + if (warn && backing_file && !backing_fmt) { + warn_report("Deprecated use of backing file without explicit " + "backing format, use of this image requires " + "potentially unsafe format probing"); } if (drv->bdrv_change_backing_file != NULL) { @@ -6873,11 +6875,24 @@ void bdrv_img_create(const char *filename, const char *fmt, goto out; } else { if (!backing_fmt) { - error_setg(&local_err, - "Backing file specified without backing format"); - error_append_hint(&local_err, "Detected format of %s.", - bs->drv->format_name); - goto out; + warn_report("Deprecated use of backing file without explicit " + "backing format (detected format of %s)", + bs->drv->format_name); + if (bs->drv != &bdrv_raw) { + /* + * A probe of raw deserves the most attention: + * leaving the backing format out of the image + * will ensure bs->probed is set (ensuring we + * don't accidentally commit into the backing + * file), and allow more spots to warn the users + * to fix their toolchain when opening this image + * later. For other images, we can safely record + * the format that we probed. + */ + backing_fmt = bs->drv->format_name; + qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, backing_fmt, + NULL); + } } if (size == -1) { /* Opened BS, have no size */ @@ -6894,9 +6909,9 @@ void bdrv_img_create(const char *filename, const char *fmt, } /* (backing_file && !(flags & BDRV_O_NO_BACKING)) */ } else if (backing_file && !backing_fmt) { - error_setg(&local_err, - "Backing file specified without backing format"); - goto out; + warn_report("Deprecated use of unopened backing file without " + "explicit backing format, use of this image requires " + "potentially unsafe format probing"); } if (size == -1) { diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index ff7488cb63b93830f75093030add..a5de09adbe9fd84b897101b31999 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -401,6 +401,29 @@ The ``I7200`` guest CPU relies on the nanoMIPS ISA, which is deprecated (the ISA has never been upstreamed to a compiler toolchain). Therefore this CPU is also deprecated. +Related binaries +---------------- + +qemu-img backing file without format (since 5.1) +'''''''''''''''''''''''''''''''''''''''''''''''' + +The use of ``qemu-img create``, ``qemu-img rebase``, or ``qemu-img +convert`` to create or modify an image that depends on a backing file +now recommends that an explicit backing format be provided. This is +for safety: if QEMU probes a different format than what you thought, +the data presented to the guest will be corrupt; similarly, presenting +a raw image to a guest allows a potential security exploit if a future +probe sees a non-raw image based on guest writes. + +To avoid the warning message, or even future refusal to create an +unsafe image, you must pass ``-o backing_fmt=`` (or the shorthand +``-F`` during create) to specify the intended backing format. You may +use ``qemu-img rebase -u`` to retroactively add a backing format to an +existing image. However, be aware that there are already potential +security risks to blindly using ``qemu-img info`` to probe the format +of an untrusted backing image, when deciding what format to add into +an existing image. + Backwards compatibility ----------------------- diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index d42c3341dee4462adb8496691575..15b34368f99fc649595507c742a1 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -670,25 +670,6 @@ backing chain should be performed with ``qemu-img rebase -u`` either before or after the remaining changes being performed by amend, as appropriate. -``qemu-img`` backing file without format (removed in 6.1) -''''''''''''''''''''''''''''''''''''''''''''''''''''''''' - -The use of ``qemu-img create``, ``qemu-img rebase``, or ``qemu-img -convert`` to create or modify an image that depends on a backing file -now requires that an explicit backing format be provided. This is -for safety: if QEMU probes a different format than what you thought, -the data presented to the guest will be corrupt; similarly, presenting -a raw image to a guest allows a potential security exploit if a future -probe sees a non-raw image based on guest writes. - -To avoid creating unsafe backing chains, you must pass ``-o -backing_fmt=`` (or the shorthand ``-F`` during create) to specify the -intended backing format. You may use ``qemu-img rebase -u`` to -retroactively add a backing format to an existing image. However, be -aware that there are already potential security risks to blindly using -``qemu-img info`` to probe the format of an untrusted backing image, -when deciding what format to add into an existing image. - Block devices ------------- diff --git a/qemu-img.c b/qemu-img.c index 6570fc67dd37a708cbe379331930..0239bdd6d09d635acee68ac28c36 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2553,10 +2553,8 @@ static int img_convert(int argc, char **argv) if (out_baseimg_param) { if (!qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT)) { - error_report("Use of backing file requires explicit " - "backing format"); - ret = -1; - goto out; + warn_report("Deprecated use of backing file without explicit " + "backing format"); } } diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index 6af5ab9e764cc5a061712a622a81..26ebe0b02f75a13c05ec5d938f53 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -915,8 +915,8 @@ class TestCommitWithOverriddenBacking(iotests.QMPTestCase): def setUp(self): qemu_img('create', '-f', iotests.imgfmt, self.img_base_a, '1M') qemu_img('create', '-f', iotests.imgfmt, self.img_base_b, '1M') - qemu_img('create', '-f', iotests.imgfmt, '-b', self.img_base_a, - '-F', iotests.imgfmt, self.img_top) + qemu_img('create', '-f', iotests.imgfmt, '-b', self.img_base_a, \ + self.img_top) self.vm = iotests.VM() self.vm.launch() diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index db9f5dc540e84c597d30521d4b6c..5cc02b24fc7a8be3d28e0e9737cb 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -1295,10 +1295,8 @@ class TestReplaces(iotests.QMPTestCase): class TestFilters(iotests.QMPTestCase): def setUp(self): qemu_img('create', '-f', iotests.imgfmt, backing_img, '1M') - qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, - '-F', iotests.imgfmt, test_img) - qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, - '-F', iotests.imgfmt, target_img) + qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, test_img) + qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, target_img) qemu_io('-c', 'write -P 1 0 512k', backing_img) qemu_io('-c', 'write -P 2 512k 512k', test_img) diff --git a/tests/qemu-iotests/114 b/tests/qemu-iotests/114 index de6fd327eea9e2ef5d21fc6fbf0b..43cb0bc6c3443a809f4dcd42ecce 100755 --- a/tests/qemu-iotests/114 +++ b/tests/qemu-iotests/114 @@ -44,16 +44,16 @@ _supported_os Linux # qcow2.py does not work too well with external data files _unsupported_imgopts data_file -# Older qemu-img could set up backing file without backing format; modern -# qemu can't but we can use qcow2.py to simulate older files. +# Intentionally specify backing file without backing format; demonstrate +# the difference in warning messages when backing file could be probed. +# Note that only a non-raw probe result will affect the resulting image. truncate -s $((64 * 1024 * 1024)) "$TEST_IMG.orig" -_make_test_img -b "$TEST_IMG.orig" -F raw 64M -$PYTHON qcow2.py "$TEST_IMG" del-header-ext 0xE2792ACA +_make_test_img -b "$TEST_IMG.orig" 64M TEST_IMG="$TEST_IMG.base" _make_test_img 64M $QEMU_IMG convert -O qcow2 -B "$TEST_IMG.orig" "$TEST_IMG.orig" "$TEST_IMG" -_make_test_img -b "$TEST_IMG.base" -F $IMGFMT 64M -_make_test_img -u -b "$TEST_IMG.base" -F $IMGFMT 64M +_make_test_img -b "$TEST_IMG.base" 64M +_make_test_img -u -b "$TEST_IMG.base" 64M # Set an invalid backing file format $PYTHON qcow2.py "$TEST_IMG" add-header-ext 0xE2792ACA "foo" @@ -64,9 +64,9 @@ _img_info $QEMU_IO -c "open $TEST_IMG" -c "read 0 4k" 2>&1 | _filter_qemu_io | _filter_testdir $QEMU_IO -c "open -o backing.driver=$IMGFMT $TEST_IMG" -c "read 0 4k" | _filter_qemu_io -# Rebase the image, to show that backing format is required. -($QEMU_IMG rebase -u -b "$TEST_IMG.base" "$TEST_IMG" 2>&1 && echo "unexpected pass") | _filter_testdir -$QEMU_IMG rebase -u -b "$TEST_IMG.base" -F $IMGFMT "$TEST_IMG" +# Rebase the image, to show that omitting backing format triggers a warning, +# but probing now lets us use the backing file. +$QEMU_IMG rebase -u -b "$TEST_IMG.base" "$TEST_IMG" $QEMU_IO -c "open $TEST_IMG" -c "read 0 4k" 2>&1 | _filter_qemu_io | _filter_testdir # success, all done diff --git a/tests/qemu-iotests/114.out b/tests/qemu-iotests/114.out index 6d638da266e495e8bed2fbcf919e..0a37d20c82a95d6d5ea9fc9635e7 100644 --- a/tests/qemu-iotests/114.out +++ b/tests/qemu-iotests/114.out @@ -1,9 +1,12 @@ QA output created by 114 -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.orig backing_fmt=raw +qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of raw) +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.orig Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 -qemu-img: Use of backing file requires explicit backing format -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT +qemu-img: warning: Deprecated use of backing file without explicit backing format +qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of IMGFMT) Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT +qemu-img: warning: Deprecated use of unopened backing file without explicit backing format, use of this image requires potentially unsafe format probing +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base image: TEST_DIR/t.IMGFMT file format: IMGFMT virtual size: 64 MiB (67108864 bytes) @@ -14,7 +17,7 @@ qemu-io: can't open device TEST_DIR/t.qcow2: Could not open backing file: Unknow no file open, try 'help open' read 4096/4096 bytes at offset 0 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -qemu-img: Could not change the backing file to 'TEST_DIR/t.qcow2.base': Invalid argument +qemu-img: warning: Deprecated use of backing file without explicit backing format, use of this image requires potentially unsafe format probing read 4096/4096 bytes at offset 0 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) *** done diff --git a/tests/qemu-iotests/301 b/tests/qemu-iotests/301 index 220de1043fa5dcad8f8563a452c1..9f943cadbe24413c7f453101a6a9 100755 --- a/tests/qemu-iotests/301 +++ b/tests/qemu-iotests/301 @@ -3,7 +3,7 @@ # # Test qcow backing file warnings # -# Copyright (C) 2020-2021 Red Hat, Inc. +# 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 @@ -46,6 +46,7 @@ echo "== qcow backed by qcow ==" TEST_IMG="$TEST_IMG.base" _make_test_img $size _make_test_img -b "$TEST_IMG.base" $size +_img_info _make_test_img -b "$TEST_IMG.base" -F $IMGFMT $size _img_info @@ -70,6 +71,7 @@ echo "== qcow backed by raw ==" rm "$TEST_IMG.base" truncate --size=$size "$TEST_IMG.base" _make_test_img -b "$TEST_IMG.base" $size +_img_info _make_test_img -b "$TEST_IMG.base" -F raw $size _img_info diff --git a/tests/qemu-iotests/301.out b/tests/qemu-iotests/301.out index e280658191e1eba861b821fafa35..9004dad6392f4a0ec685fb4a80d1 100644 --- a/tests/qemu-iotests/301.out +++ b/tests/qemu-iotests/301.out @@ -2,7 +2,13 @@ QA output created by 301 == qcow backed by qcow == Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432 -qemu-img: TEST_DIR/t.IMGFMT: Backing file specified without backing format +qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of IMGFMT) +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT +image: TEST_DIR/t.IMGFMT +file format: IMGFMT +virtual size: 32 MiB (33554432 bytes) +cluster_size: 512 +backing file: TEST_DIR/t.IMGFMT.base Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT image: TEST_DIR/t.IMGFMT file format: IMGFMT @@ -30,7 +36,13 @@ cluster_size: 512 backing file: TEST_DIR/t.IMGFMT.base == qcow backed by raw == -qemu-img: TEST_DIR/t.IMGFMT: Backing file specified without backing format +qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of raw) +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base +image: TEST_DIR/t.IMGFMT +file format: IMGFMT +virtual size: 32 MiB (33554432 bytes) +cluster_size: 512 +backing file: TEST_DIR/t.IMGFMT.base Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw image: TEST_DIR/t.IMGFMT file format: IMGFMT