12:58 p.m.
Creating an image that requires format probing of the backing image is
inherently unsafe (we've had several CVEs over the years based on
probes leaking information to the guest on a subsequent boot, although
these days tools like libvirt are aware of the issue enough to prevent
the worst effects). However, if our probing algorithm ever changes,
or if other tools like libvirt determine a different probe result than
we do, then subsequent use of that backing file under a different
format will present corrupted data to the guest. Start a deprecation
clock so that future qemu-img can refuse to create unsafe backing
chains that would rely on probing. Most warnings are intentionally
emitted from bdrv_img_create() in the block layer, but qemu-img
convert uses bdrv_create() which cannot emit its own warning without
causing spurious warnings on other code paths. In the end, all
command-line image creation or backing file rewriting now performs a
check.
However, there is one time where probing is safe: if we probe raw,
then it is safe to record that implicitly in the image (but we still
warn, as it's better to teach the user to supply -F always than to
make them guess when it is safe).
iotest 114 specifically wants to create an unsafe image for later
amendment rather than defaulting to our new default of recording a
probed format, so it needs an update. While touching it, expand it to
cover all of the various warnings enabled by this patch. iotest 290
also shows a change to qcow messages; note that the fact that we now
make a probed format of 'raw' explicit now results in a double
warning, but no one should be creating new qcow images so it is not
worth cleaning up.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/system/deprecated.rst | 20 ++++++++++++++++++++
block.c | 21 ++++++++++++++++++++-
qemu-img.c | 9 ++++++++-
tests/qemu-iotests/114 | 12 ++++++++++++
tests/qemu-iotests/114.out | 9 +++++++++
tests/qemu-iotests/290.out | 5 ++++-
6 files changed, 73 insertions(+), 3 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index aac6be58917a..8abfd436820a 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -429,6 +429,26 @@ 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.0.0)
+''''''''''''''''''''''''''''''''''''''''''''''''''
+
+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.
+
``qemu-img convert -n -o`` (since 4.2.0)
''''''''''''''''''''''''''''''''''''''''
diff --git a/block.c b/block.c
index 70cc6123dd91..61871965eee4 100644
--- a/block.c
+++ b/block.c
@@ -6072,6 +6072,20 @@ void bdrv_img_create(const char *filename, const char *fmt,
"Could not open backing image to determine
size.\n");
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 is always correct, so in this one
+ * case, we can write that into the image.
+ */
+ 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 */
size = bdrv_getlength(bs);
@@ -6085,7 +6099,12 @@ void bdrv_img_create(const char *filename, const char *fmt,
}
bdrv_unref(bs);
}
- } /* (backing_file && !(flags & BDRV_O_NO_BACKING)) */
+ /* (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");
+ }
if (size == -1) {
error_setg(errp, "Image creation needs a size parameter");
diff --git a/qemu-img.c b/qemu-img.c
index cb8fa5a0ee8b..fec89da4e18f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2414,6 +2414,13 @@ static int img_convert(int argc, char **argv)
goto out;
}
+ if (out_baseimg_param) {
+ if (!qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT)) {
+ warn_report("Deprecated use of backing file without explicit "
+ "backing format");
+ }
+ }
+
/* Check if compression is supported */
if (s.compressed) {
bool encryption =
@@ -3644,7 +3651,7 @@ static int img_rebase(int argc, char **argv)
* doesn't change when we switch the backing file.
*/
if (out_baseimg && *out_baseimg) {
- ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt, false);
+ ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt, true);
} else {
ret = bdrv_change_backing_file(bs, NULL, NULL, false);
}
diff --git a/tests/qemu-iotests/114 b/tests/qemu-iotests/114
index 26104fff6c67..e65bd4f983e0 100755
--- a/tests/qemu-iotests/114
+++ b/tests/qemu-iotests/114
@@ -42,9 +42,16 @@ _unsupported_proto vxhs
# 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 raw probe result will affect the resulting image.
+truncate --size=64M "$TEST_IMG.orig"
+_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" 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"
@@ -55,6 +62,11 @@ _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"
+$QEMU_IO -c "open $TEST_IMG" -c "read 0 4k" 2>&1 |
_filter_qemu_io | _filter_testdir
+
# success, all done
echo '*** done'
rm -f $seq.full
diff --git a/tests/qemu-iotests/114.out b/tests/qemu-iotests/114.out
index 67adef37a4f6..7c15dd53b0b5 100644
--- a/tests/qemu-iotests/114.out
+++ b/tests/qemu-iotests/114.out
@@ -1,5 +1,11 @@
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 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)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
backing_file=TEST_DIR/t.IMGFMT.base
+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
@@ -11,4 +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: 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/290.out b/tests/qemu-iotests/290.out
index 5b4ae3196d04..f80a5d982726 100644
--- a/tests/qemu-iotests/290.out
+++ b/tests/qemu-iotests/290.out
@@ -2,6 +2,7 @@ QA output created by 290
== qcow backed by qcow ==
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+qemu-img: warning: Deprecated use of backing file without explicit backing format
(detected format of IMGFMT)
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
backing_file=TEST_DIR/t.IMGFMT.base
image: TEST_DIR/t.IMGFMT
file format: IMGFMT
@@ -26,7 +27,9 @@ 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
+qemu-img: warning: Deprecated use of backing file without explicit backing format
(detected format of raw)
+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)
--
2.26.0.rc2