Skip to content

Commit

Permalink
qemu-img: Require -F with -b backing image
Browse files Browse the repository at this point in the history
Back in commit d9f059a (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>
Message-Id: <20210503213600.569128-3-eblake@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
ebblake authored and kevmw committed Jul 9, 2021
1 parent 5a385bf commit 497a30d
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 85 deletions.
37 changes: 11 additions & 26 deletions block.c
Expand Up @@ -5074,7 +5074,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;
Expand All @@ -5088,10 +5088,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) {
Expand Down Expand Up @@ -6601,24 +6599,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 */
Expand All @@ -6635,9 +6620,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) {
Expand Down
20 changes: 0 additions & 20 deletions docs/system/deprecated.rst
Expand Up @@ -282,26 +282,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
-----------------------

Expand Down
19 changes: 19 additions & 0 deletions docs/system/removed-features.rst
Expand Up @@ -503,6 +503,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
-------------

Expand Down
6 changes: 4 additions & 2 deletions qemu-img.c
Expand Up @@ -2508,8 +2508,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;
}
}

Expand Down
4 changes: 2 additions & 2 deletions tests/qemu-iotests/040
Expand Up @@ -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()
Expand Down
6 changes: 4 additions & 2 deletions tests/qemu-iotests/041
Expand Up @@ -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)
Expand Down
18 changes: 9 additions & 9 deletions tests/qemu-iotests/114
Expand Up @@ -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"
Expand All @@ -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" 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

# success, all done
Expand Down
11 changes: 4 additions & 7 deletions 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)
Expand All @@ -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 'TEST_DIR/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
4 changes: 1 addition & 3 deletions tests/qemu-iotests/301
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down
16 changes: 2 additions & 14 deletions tests/qemu-iotests/301.out
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 497a30d

Please sign in to comment.