[PATCH 0/2] Remove deprecated qemu-img backing file without format

[PATCH 0/2] Remove deprecated qemu-img backing file without format

[Eric Blake]

We've gone enough release cycles without noticeable pushback on our
intentions, so time to make it harder to create images that can form a
security hole due to a need for format probing rather than an explicit
format.

Eric Blake (2):
  qcow2: Prohibit backing file changes in 'qemu-img amend'
  qemu-img: Require -F with -b backing image

 docs/system/deprecated.rst       | 32 ---------------------------
 docs/system/removed-features.rst | 31 ++++++++++++++++++++++++++
 block.c                          | 37 ++++++++++----------------------
 block/qcow2.c                    | 13 ++++-------
 qemu-img.c                       |  6 ++++--
 tests/qemu-iotests/061           |  3 +++
 tests/qemu-iotests/061.out       |  3 ++-
 tests/qemu-iotests/082.out       |  6 ++++--
 tests/qemu-iotests/114           | 18 ++++++++--------
 tests/qemu-iotests/114.out       | 11 ++++------
 tests/qemu-iotests/301           |  4 +---
 tests/qemu-iotests/301.out       | 16 ++------------
 12 files changed, 75 insertions(+), 105 deletions(-)

-- 
2.31.1

[PATCH 1/2] qcow2: Prohibit backing file changes in 'qemu-img amend'

[Eric Blake]

This was deprecated back in bc5ee6da7 (qcow2: Deprecate use of
qemu-img amend to change backing file), and no one in the meantime has
given any reasons why it should be supported.  Time to make change
attempts a hard error (but for convenience, specifying the _same_
backing chain is not forbidden).  Update a couple of iotests to match.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 docs/system/deprecated.rst       | 12 ------------
 docs/system/removed-features.rst | 12 ++++++++++++
 block/qcow2.c                    | 13 ++++---------
 tests/qemu-iotests/061           |  3 +++
 tests/qemu-iotests/061.out       |  3 ++-
 tests/qemu-iotests/082.out       |  6 ++++--
 6 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 80cae862528a..9ec1a9d0e03e 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -315,18 +315,6 @@ this CPU is also deprecated.
 Related binaries
 ----------------

-qemu-img amend to adjust backing file (since 5.1)
-'''''''''''''''''''''''''''''''''''''''''''''''''
-
-The use of ``qemu-img amend`` to modify the name or format of a qcow2
-backing image is deprecated; this functionality was never fully
-documented or tested, and interferes with other amend operations that
-need access to the original backing image (such as deciding whether a
-v3 zero cluster may be left unallocated when converting to a v2
-image).  Rather, any changes to the 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 (since 5.1)
 ''''''''''''''''''''''''''''''''''''''''''''''''

diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 29e90601a51a..28b5df757d35 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -454,6 +454,18 @@ topologies described with -smp include all possible cpus, i.e.
 The ``enforce-config-section`` property was replaced by the
 ``-global migration.send-configuration={on|off}`` option.

+qemu-img amend to adjust backing file (removed in 6.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The use of ``qemu-img amend`` to modify the name or format of a qcow2
+backing image was never fully documented or tested, and interferes
+with other amend operations that need access to the original backing
+image (such as deciding whether a v3 zero cluster may be left
+unallocated when converting to a v2 image).  Any changes to the
+backing chain should be performed with ``qemu-img rebase -u`` either
+before or after the remaining changes being performed by amend, as
+appropriate.
+
 Block devices
 -------------

diff --git a/block/qcow2.c b/block/qcow2.c
index 9727ae8fe341..898c4fc4464c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5620,15 +5620,10 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
     if (backing_file || backing_format) {
         if (g_strcmp0(backing_file, s->image_backing_file) ||
             g_strcmp0(backing_format, s->image_backing_format)) {
-            warn_report("Deprecated use of amend to alter the backing file; "
-                        "use qemu-img rebase instead");
-        }
-        ret = qcow2_change_backing_file(bs,
-                    backing_file ?: s->image_backing_file,
-                    backing_format ?: s->image_backing_format);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Failed to change the backing file");
-            return ret;
+            error_setg(errp, "Cannot amend the backing file");
+            error_append_hint(errp,
+                              "You can use 'qemu-img rebase' instead.\n");
+            return -EINVAL;
         }
     }

diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
index e26d94a0df31..9507c223bda4 100755
--- a/tests/qemu-iotests/061
+++ b/tests/qemu-iotests/061
@@ -167,6 +167,9 @@ _make_test_img -o "compat=1.1" 64M
 TEST_IMG="$TEST_IMG.base" _make_test_img -o "compat=1.1" 64M
 $QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG.base" | _filter_qemu_io
 $QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG amend -o "backing_file=$TEST_IMG.base,backing_fmt=qcow2" \
+	  "$TEST_IMG" && echo "unexpected pass"
+$QEMU_IMG rebase -u -b "$TEST_IMG.base" -F qcow2 "$TEST_IMG"
 $QEMU_IMG amend -o "backing_file=$TEST_IMG.base,backing_fmt=qcow2" "$TEST_IMG"
 $QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
 _check_test_img
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index ee30da266514..7ecbd4dea875 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -370,7 +370,8 @@ wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-qemu-img: warning: Deprecated use of amend to alter the backing file; use qemu-img rebase instead
+qemu-img: Cannot amend the backing file
+You can use 'qemu-img rebase' instead.
 read 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 No errors were found on the image.
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index b70c12c1393d..077ed0f2c7d9 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -808,12 +808,14 @@ Amend options for 'qcow2':
   size=<size>            - Virtual disk size

 Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2
-qemu-img: warning: Deprecated use of amend to alter the backing file; use qemu-img rebase instead
+qemu-img: Cannot amend the backing file
+You can use 'qemu-img rebase' instead.

 Testing: rebase -u -b  -f qcow2 TEST_DIR/t.qcow2

 Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,? TEST_DIR/t.qcow2
-qemu-img: warning: Deprecated use of amend to alter the backing file; use qemu-img rebase instead
+qemu-img: Cannot amend the backing file
+You can use 'qemu-img rebase' instead.

 Testing: rebase -u -b  -f qcow2 TEST_DIR/t.qcow2

-- 
2.31.1

[PATCH 2/2] qemu-img: Require -F with -b backing image

[Eric Blake]

Back in commit d9f059aa6c (qemu-img: Deprecate use of -b without -F),
we deprecated the ability to create a file with a backing image that
requires qemu to perform format probing.  Qemu can still probe older
files for backwards compatibility, but it is time to finish off the
ability to create such images, due to the potential security risk they
present.  Update a couple of iotests affected by the change.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 docs/system/deprecated.rst       | 20 -----------------
 docs/system/removed-features.rst | 19 ++++++++++++++++
 block.c                          | 37 ++++++++++----------------------
 qemu-img.c                       |  6 ++++--
 tests/qemu-iotests/114           | 18 ++++++++--------
 tests/qemu-iotests/114.out       | 11 ++++------
 tests/qemu-iotests/301           |  4 +---
 tests/qemu-iotests/301.out       | 16 ++------------
 8 files changed, 50 insertions(+), 81 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 9ec1a9d0e03e..aa6f7d84e583 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -315,26 +315,6 @@ 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/system/removed-features.rst b/docs/system/removed-features.rst
index 28b5df757d35..1928d8a483c0 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -466,6 +466,25 @@ 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/block.c b/block.c
index 874c22c43e3d..931e37a8499b 100644
--- a/block.c
+++ b/block.c
@@ -5033,7 +5033,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 warn)
+                             const char *backing_fmt, bool require)
 {
     BlockDriver *drv = bs->drv;
     int ret;
@@ -5047,10 +5047,8 @@ int bdrv_change_backing_file(BlockDriverState *bs, const char *backing_file,
         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 (require && backing_file && !backing_fmt) {
+        return -EINVAL;
     }

     if (drv->bdrv_change_backing_file != NULL) {
@@ -6556,24 +6554,11 @@ void bdrv_img_create(const char *filename, const char *fmt,
             goto out;
         } else {
             if (!backing_fmt) {
-                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);
-                }
+                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;
             }
             if (size == -1) {
                 /* Opened BS, have no size */
@@ -6590,9 +6575,9 @@ void bdrv_img_create(const char *filename, const char *fmt,
         }
         /* (backing_file && !(flags & BDRV_O_NO_BACKING)) */
     } else if (backing_file && !backing_fmt) {
-        warn_report("Deprecated use of unopened backing file without "
-                    "explicit backing format, use of this image requires "
-                    "potentially unsafe format probing");
+        error_setg(&local_err,
+                   "Backing file specified without backing format");
+        goto out;
     }

     if (size == -1) {
diff --git a/qemu-img.c b/qemu-img.c
index a5993682aad4..3cdfcaa23f6b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2505,8 +2505,10 @@ static int img_convert(int argc, char **argv)

     if (out_baseimg_param) {
         if (!qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT)) {
-            warn_report("Deprecated use of backing file without explicit "
-                        "backing format");
+            error_report("Use of backing file requires explicit "
+                         "backing format");
+            ret = -1;
+            goto out;
         }
     }

diff --git a/tests/qemu-iotests/114 b/tests/qemu-iotests/114
index 43cb0bc6c344..3e30b402bc4c 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

-# 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.
+# 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.
 truncate -s $((64 * 1024 * 1024)) "$TEST_IMG.orig"
-_make_test_img -b "$TEST_IMG.orig" 64M
+_make_test_img -b "$TEST_IMG.orig" -F raw 64M
+$PYTHON qcow2.py "$TEST_IMG" del-header-ext 0xE2792ACA

 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" 64M
-_make_test_img -u -b "$TEST_IMG.base" 64M
+_make_test_img -b "$TEST_IMG.base" -F $IMGFMT 64M
+_make_test_img -u -b "$TEST_IMG.base" -F $IMGFMT 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 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"
+# Rebase the image, to show that backing format is required.
+$QEMU_IMG rebase -u -b "$TEST_IMG.base" "$TEST_IMG" && echo "unexpected pass"
+$QEMU_IMG rebase -u -b "$TEST_IMG.base" -F $IMGFMT "$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 0a37d20c82a9..172454401257 100644
--- a/tests/qemu-iotests/114.out
+++ b/tests/qemu-iotests/114.out
@@ -1,12 +1,9 @@
 QA output created by 114
-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', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.orig backing_fmt=raw
 Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
-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)
+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
 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)
@@ -17,7 +14,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: warning: Deprecated use of backing file without explicit backing format, use of this image requires potentially unsafe format probing
+qemu-img: Could not change the backing file to '/home/eblake/qemu/build/tests/qemu-iotests/scratch/t.qcow2.base': Invalid argument
 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 9f943cadbe24..220de1043fa5 100755
--- a/tests/qemu-iotests/301
+++ b/tests/qemu-iotests/301
@@ -3,7 +3,7 @@
 #
 # Test qcow backing file warnings
 #
-# Copyright (C) 2020 Red Hat, Inc.
+# Copyright (C) 2020-2021 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,7 +46,6 @@ 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

@@ -71,7 +70,6 @@ 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 9004dad6392f..e280658191e1 100644
--- a/tests/qemu-iotests/301.out
+++ b/tests/qemu-iotests/301.out
@@ -2,13 +2,7 @@ QA output created by 301

 == qcow backed by qcow ==
 Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432
-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
+qemu-img: TEST_DIR/t.IMGFMT: Backing file specified without backing format
 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
@@ -36,13 +30,7 @@ cluster_size: 512
 backing file: TEST_DIR/t.IMGFMT.base

 == qcow backed by 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=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
+qemu-img: TEST_DIR/t.IMGFMT: Backing file specified without backing format
 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
-- 
2.31.1

Re: [PATCH 2/2] qemu-img: Require -F with -b backing image

[Eric Blake]

On 5/3/21 4:36 PM, Eric Blake wrote:
> Back in commit d9f059aa6c (qemu-img: Deprecate use of -b without -F),
> we deprecated the ability to create a file with a backing image that
> requires qemu to perform format probing.  Qemu can still probe older
> files for backwards compatibility, but it is time to finish off the
> ability to create such images, due to the potential security risk they
> present.  Update a couple of iotests affected by the change.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  docs/system/deprecated.rst       | 20 -----------------
>  docs/system/removed-features.rst | 19 ++++++++++++++++
>  block.c                          | 37 ++++++++++----------------------
>  qemu-img.c                       |  6 ++++--
>  tests/qemu-iotests/114           | 18 ++++++++--------
>  tests/qemu-iotests/114.out       | 11 ++++------
>  tests/qemu-iotests/301           |  4 +---
>  tests/qemu-iotests/301.out       | 16 ++------------
>  8 files changed, 50 insertions(+), 81 deletions(-)

I'll need a followup to fix iotest failures in 40 and 41 (apparently
they weren't passing backing formats, but I did not catch them in my
original cleanup of iotests back in commit b66ff2c298)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 2/2] qemu-img: Require -F with -b backing image

[Eric Blake]

On 5/3/21 4:45 PM, Eric Blake wrote:
> On 5/3/21 4:36 PM, Eric Blake wrote:
>> Back in commit d9f059aa6c (qemu-img: Deprecate use of -b without -F),
>> we deprecated the ability to create a file with a backing image that
>> requires qemu to perform format probing.  Qemu can still probe older
>> files for backwards compatibility, but it is time to finish off the
>> ability to create such images, due to the potential security risk they
>> present.  Update a couple of iotests affected by the change.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>  docs/system/deprecated.rst       | 20 -----------------
>>  docs/system/removed-features.rst | 19 ++++++++++++++++
>>  block.c                          | 37 ++++++++++----------------------
>>  qemu-img.c                       |  6 ++++--
>>  tests/qemu-iotests/114           | 18 ++++++++--------
>>  tests/qemu-iotests/114.out       | 11 ++++------
>>  tests/qemu-iotests/301           |  4 +---
>>  tests/qemu-iotests/301.out       | 16 ++------------
>>  8 files changed, 50 insertions(+), 81 deletions(-)
> 
> I'll need a followup to fix iotest failures in 40 and 41 (apparently
> they weren't passing backing formats, but I did not catch them in my
> original cleanup of iotests back in commit b66ff2c298)

Squash in:

diff --git i/tests/qemu-iotests/040 w/tests/qemu-iotests/040
index ba7cb34ce8cf..f3677de9dfde 100755
--- i/tests/qemu-iotests/040
+++ w/tests/qemu-iotests/040
@@ -920,8 +920,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, \
-                 self.img_top)
+        qemu_img('create', '-f', iotests.imgfmt, '-b', self.img_base_a,
+                 '-F', iotests.imgfmt, self.img_top)

         self.vm = iotests.VM()
         self.vm.launch()
diff --git i/tests/qemu-iotests/041 w/tests/qemu-iotests/041
index 5cc02b24fc7a..db9f5dc540e8 100755
--- i/tests/qemu-iotests/041
+++ w/tests/qemu-iotests/041
@@ -1295,8 +1295,10 @@ 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,
test_img)
-        qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img,
target_img)
+        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_io('-c', 'write -P 1 0 512k', backing_img)
         qemu_io('-c', 'write -P 2 512k 512k', test_img)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 0/2] Remove deprecated qemu-img backing file without format

[Eric Blake]

On Mon, May 03, 2021 at 04:35:58PM -0500, Eric Blake wrote:
> We've gone enough release cycles without noticeable pushback on our
> intentions, so time to make it harder to create images that can form a
> security hole due to a need for format probing rather than an explicit
> format.
> 
> Eric Blake (2):
>   qcow2: Prohibit backing file changes in 'qemu-img amend'
>   qemu-img: Require -F with -b backing image

Ping.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 0/2] Remove deprecated qemu-img backing file without format

[Connor Kuehl]

On 5/3/21 2:35 PM, Eric Blake wrote:
> We've gone enough release cycles without noticeable pushback on our
> intentions, so time to make it harder to create images that can form a
> security hole due to a need for format probing rather than an explicit
> format.
> 
> Eric Blake (2):
>   qcow2: Prohibit backing file changes in 'qemu-img amend'
>   qemu-img: Require -F with -b backing image
> 
>  docs/system/deprecated.rst       | 32 ---------------------------
>  docs/system/removed-features.rst | 31 ++++++++++++++++++++++++++
>  block.c                          | 37 ++++++++++----------------------
>  block/qcow2.c                    | 13 ++++-------
>  qemu-img.c                       |  6 ++++--
>  tests/qemu-iotests/061           |  3 +++
>  tests/qemu-iotests/061.out       |  3 ++-
>  tests/qemu-iotests/082.out       |  6 ++++--
>  tests/qemu-iotests/114           | 18 ++++++++--------
>  tests/qemu-iotests/114.out       | 11 ++++------
>  tests/qemu-iotests/301           |  4 +---
>  tests/qemu-iotests/301.out       | 16 ++------------
>  12 files changed, 75 insertions(+), 105 deletions(-)
> 

For the series + the squash attached to patch 2:

Reviewed-by: Connor Kuehl <ckuehl@redhat.com>

Re: [PATCH 2/2] qemu-img: Require -F with -b backing image

[Kevin Wolf]

Am 03.05.2021 um 23:36 hat Eric Blake geschrieben:
> @@ -17,7 +14,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: warning: Deprecated use of backing file without explicit backing format, use of this image requires potentially unsafe format probing
> +qemu-img: Could not change the backing file to '/home/eblake/qemu/build/tests/qemu-iotests/scratch/t.qcow2.base': Invalid argument
>  read 4096/4096 bytes at offset 0
>  4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

This is not exactly an improvement for the error message. Maybe worth a
follow-up patch?

Kevin

Re: [PATCH 0/2] Remove deprecated qemu-img backing file without format

[Kevin Wolf]

Am 07.07.2021 um 23:17 hat Eric Blake geschrieben:
> On Mon, May 03, 2021 at 04:35:58PM -0500, Eric Blake wrote:
> > We've gone enough release cycles without noticeable pushback on our
> > intentions, so time to make it harder to create images that can form a
> > security hole due to a need for format probing rather than an explicit
> > format.
> > 
> > Eric Blake (2):
> >   qcow2: Prohibit backing file changes in 'qemu-img amend'
> >   qemu-img: Require -F with -b backing image
> 
> Ping.

Thanks, applied to the block branch.

For some reason, the CCs were missing on the cover letter. Please make
sure to CC me (and qemu-block) for the whole series if you want it to go
through my tree.

Kevin

Re: [PATCH 2/2] qemu-img: Require -F with -b backing image

[Eric Blake]

On Thu, Jul 08, 2021 at 03:00:36PM +0200, Kevin Wolf wrote:
> Am 03.05.2021 um 23:36 hat Eric Blake geschrieben:
> > @@ -17,7 +14,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: warning: Deprecated use of backing file without explicit backing format, use of this image requires potentially unsafe format probing
> > +qemu-img: Could not change the backing file to '/home/eblake/qemu/build/tests/qemu-iotests/scratch/t.qcow2.base': Invalid argument
> >  read 4096/4096 bytes at offset 0
> >  4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> 
> This is not exactly an improvement for the error message. Maybe worth a
> follow-up patch?

Sure, will do.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[PATCH 3/2] qemu-img: Improve error for rebase without backing format

[Eric Blake]

When removeing support for qemu-img being able to create backing
chains without embedded backing formats, we caused a poor error
message as caught by iotest 114.  Improve the situation to inform the
user what went wrong.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qemu-img.c                 | 3 +++
 tests/qemu-iotests/114.out | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 7c0e73882dd4..b017734c255a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3786,6 +3786,9 @@ static int img_rebase(int argc, char **argv)
     if (ret == -ENOSPC) {
         error_report("Could not change the backing file to '%s': No "
                      "space left in the file header", out_baseimg);
+    } else if (ret == -EINVAL && out_baseimg && !out_basefmt) {
+        error_report("Could not change the backing file to '%s': backing "
+                     "format must be specified", out_baseimg);
     } else if (ret < 0) {
         error_report("Could not change the backing file to '%s': %s",
             out_baseimg, strerror(-ret));
diff --git a/tests/qemu-iotests/114.out b/tests/qemu-iotests/114.out
index 172454401257..016e9ce3ecfb 100644
--- a/tests/qemu-iotests/114.out
+++ b/tests/qemu-iotests/114.out
@@ -14,7 +14,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 '/home/eblake/qemu/build/tests/qemu-iotests/scratch/t.qcow2.base': Invalid argument
+qemu-img: Could not change the backing file to '/home/eblake/qemu/build/tests/qemu-iotests/scratch/t.qcow2.base': backing format must be specified
 read 4096/4096 bytes at offset 0
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 *** done
-- 
2.31.1

Re: [PATCH 3/2] qemu-img: Improve error for rebase without backing format

[Kevin Wolf]

Am 08.07.2021 um 17:52 hat Eric Blake geschrieben:
> When removeing support for qemu-img being able to create backing
> chains without embedded backing formats, we caused a poor error
> message as caught by iotest 114.  Improve the situation to inform the
> user what went wrong.
> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Thanks, applied to the block branch.

Kevin

Re: [PATCH 3/2] qemu-img: Improve error for rebase without backing format

[Kevin Wolf]

Am 08.07.2021 um 17:52 hat Eric Blake geschrieben:
> When removeing support for qemu-img being able to create backing
> chains without embedded backing formats, we caused a poor error
> message as caught by iotest 114.  Improve the situation to inform the
> user what went wrong.
> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>

> diff --git a/tests/qemu-iotests/114.out b/tests/qemu-iotests/114.out
> index 172454401257..016e9ce3ecfb 100644
> --- a/tests/qemu-iotests/114.out
> +++ b/tests/qemu-iotests/114.out
> @@ -14,7 +14,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 '/home/eblake/qemu/build/tests/qemu-iotests/scratch/t.qcow2.base': Invalid argument
> +qemu-img: Could not change the backing file to '/home/eblake/qemu/build/tests/qemu-iotests/scratch/t.qcow2.base': backing format must be specified
>  read 4096/4096 bytes at offset 0
>  4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  *** done

Wait, there is a problem in the image path...

I'm squashing in the following for patch 2, and will do the obvious
conflict resolution for this one.

Kevin

diff --git a/tests/qemu-iotests/114 b/tests/qemu-iotests/114
index 3e30b402bc..de6fd327ee 100755
--- a/tests/qemu-iotests/114
+++ b/tests/qemu-iotests/114
@@ -65,7 +65,7 @@ $QEMU_IO -c "open $TEST_IMG" -c "read 0 4k" 2>&1 | _filter_qemu_io | _filter_tes
 $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" && echo "unexpected pass"
+($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"
 $QEMU_IO -c "open $TEST_IMG" -c "read 0 4k" 2>&1 | _filter_qemu_io | _filter_testdir

diff --git a/tests/qemu-iotests/114.out b/tests/qemu-iotests/114.out
index 1724544012..f51dd9d20a 100644
--- a/tests/qemu-iotests/114.out
+++ b/tests/qemu-iotests/114.out
@@ -14,7 +14,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 '/home/eblake/qemu/build/tests/qemu-iotests/scratch/t.qcow2.base': Invalid argument
+qemu-img: Could not change the backing file to 'TEST_DIR/t.qcow2.base': backing format must be specified
 read 4096/4096 bytes at offset 0
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 *** done

Re: [PATCH 3/2] qemu-img: Improve error for rebase without backing format

[Eric Blake]

On Thu, Jul 08, 2021 at 06:58:21PM +0200, Kevin Wolf wrote:
> Am 08.07.2021 um 17:52 hat Eric Blake geschrieben:
> > When removeing support for qemu-img being able to create backing

removing

(is it bad that I don't catch my own typos until seeing them through
the mailing list?)

> > chains without embedded backing formats, we caused a poor error
> > message as caught by iotest 114.  Improve the situation to inform the
> > user what went wrong.
> > 
> > Suggested-by: Kevin Wolf <kwolf@redhat.com>
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> Thanks, applied to the block branch.

And thanks for catching my obvious lack of filtering out local file names.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org