qcow2: Forbid discard in qcow2 v2 images with backing files

A discard request deallocates the selected clusters so they read back
as zeroes. This is done by clearing the cluster offset field and
setting QCOW_OFLAG_ZERO in the L2 entry.

This flag is however only supported when qcow_version >= 3. In older
images the cluster is simply deallocated, exposing any possible stale
data from the backing file.

Since discard is an advisory operation it's safer to simply forbid it
in this scenario.

Note that we are adding this check to qcow2_co_pdiscard() and not to
qcow2_cluster_discard() or discard_in_l2_slice() because the last
two are also used by qcow2_snapshot_create() to discard the clusters
used by the VM state. In this case there's no risk of exposing stale
data to the guest and we really want that the clusters are always
discarded.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20200331114345.29993-1-berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
This commit is contained in:
Alberto Garcia 2020-03-31 13:43:45 +02:00 committed by Max Reitz
parent 53ef8a92eb
commit 80f5c01183
9 changed files with 187 additions and 19 deletions

View file

@ -3784,6 +3784,12 @@ static coroutine_fn int qcow2_co_pdiscard(BlockDriverState *bs,
int ret;
BDRVQcow2State *s = bs->opaque;
/* If the image does not support QCOW_OFLAG_ZERO then discarding
* clusters could expose stale data from the backing file. */
if (s->qcow_version < 3 && bs->backing) {
return -ENOTSUP;
}
if (!QEMU_IS_ALIGNED(offset | bytes, s->cluster_size)) {
assert(bytes < s->cluster_size);
/* Ignore partial clusters, except for the special case of the

View file

@ -193,8 +193,8 @@ echo "== Verify image content =="
verify_io()
{
if ($QEMU_IMG info -U -f "$IMGFMT" "$TEST_IMG" | grep "compat: 0.10" > /dev/null); then
# For v2 images, discarded clusters are read from the backing file
# Keep the variable empty so that the backing file value can be used as
# In v2 images clusters are not discarded when there is a backing file.
# Keep the variable empty so that the previous value can be used as
# the default below
discarded=
else
@ -230,14 +230,16 @@ verify_io()
echo read -P 70 0x78000 0x6000
echo read -P 7 0x7e000 0x2000
echo read -P ${discarded:-8} 0x80000 0x6000
echo read -P ${discarded:-89} 0x80000 0x1000
echo read -P ${discarded:-8} 0x81000 0x5000
echo read -P 80 0x86000 0x2000
echo read -P ${discarded:-8} 0x88000 0x2000
echo read -P 81 0x8a000 0xe000
echo read -P 90 0x98000 0x6000
echo read -P 9 0x9e000 0x2000
echo read -P ${discarded:-10} 0xa0000 0x6000
echo read -P ${discarded:-109} 0xa0000 0x1000
echo read -P ${discarded:-10} 0xa1000 0x5000
echo read -P 100 0xa6000 0x2000
echo read -P ${discarded:-10} 0xa8000 0x2000
echo read -P 101 0xaa000 0xe000

View file

@ -187,8 +187,10 @@ read 24576/24576 bytes at offset 491520
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 516096
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 24576/24576 bytes at offset 524288
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 4096/4096 bytes at offset 524288
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 20480/20480 bytes at offset 528384
20 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 548864
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 557056
@ -199,8 +201,10 @@ read 24576/24576 bytes at offset 622592
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 647168
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 24576/24576 bytes at offset 655360
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 4096/4096 bytes at offset 655360
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 20480/20480 bytes at offset 659456
20 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 679936
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 688128

View file

@ -160,18 +160,16 @@ TEST_IMG=$BACKING_IMG _make_test_img 1G
$QEMU_IO -c 'write 0k 64k' "$BACKING_IMG" | _filter_qemu_io
# compat=0.10 is required in order to make the following discard actually
# unallocate the sector rather than make it a zero sector - we want COW, after
# all.
_make_test_img -o 'compat=0.10' -b "$BACKING_IMG" 1G
_make_test_img -b "$BACKING_IMG" 1G
# Write two clusters, the second one enforces creation of an L2 table after
# the first data cluster.
$QEMU_IO -c 'write 0k 64k' -c 'write 512M 64k' "$TEST_IMG" | _filter_qemu_io
# Discard the first cluster. This cluster will soon enough be reallocated and
# Free the first cluster. This cluster will soon enough be reallocated and
# used for COW.
$QEMU_IO -c 'discard 0k 64k' "$TEST_IMG" | _filter_qemu_io
poke_file "$TEST_IMG" "$l2_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
poke_file "$TEST_IMG" "$(($rb_offset+10))" "\x00\x00"
# Now, corrupt the image by marking the second L2 table cluster as free.
poke_file "$TEST_IMG" '131084' "\x00\x00" # 0x2000c
poke_file "$TEST_IMG" "$(($rb_offset+12))" "\x00\x00"
# Start a write operation requiring COW on the image stopping it right before
# doing the read; then, trigger the corruption prevention by writing anything to
# any unallocated cluster, leading to an attempt to overwrite the second L2

View file

@ -105,8 +105,6 @@ wrote 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 536870912
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
discard 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qcow2: Marking image as corrupt: Preventing invalid write on metadata (overlaps with active L2 table); further corruption events will be suppressed
blkdebug: Suspended request '0'
write failed: Input/output error

View file

@ -89,8 +89,9 @@ verify_io()
{
if ($QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" |
grep "compat: 0.10" > /dev/null); then
# For v2 images, discarded clusters are read from the backing file
discarded=11
# In v2 images clusters are not discarded when there is a backing file
# so the previous value is read
discarded=22
else
# Discarded clusters are zeroed for v3 or later
discarded=0

97
tests/qemu-iotests/290 Executable file
View file

@ -0,0 +1,97 @@
#!/usr/bin/env bash
#
# Test how 'qemu-io -c discard' behaves on v2 and v3 qcow2 images
#
# Copyright (C) 2020 Igalia, S.L.
# Author: Alberto Garcia <berto@igalia.com>
#
# 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/>.
#
# creator
owner=berto@igalia.com
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 qcow2
_supported_proto file
_supported_os Linux
_unsupported_imgopts 'compat=0.10' refcount_bits data_file
echo
echo "### Test 'qemu-io -c discard' on a QCOW2 image without a backing file"
echo
for qcow2_compat in 0.10 1.1; do
echo "# Create an image with compat=$qcow2_compat without a backing file"
_make_test_img -o "compat=$qcow2_compat" 128k
echo "# Fill all clusters with data and then discard them"
$QEMU_IO -c 'write -P 0x01 0 128k' "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c 'discard 0 128k' "$TEST_IMG" | _filter_qemu_io
echo "# Read the data from the discarded clusters"
$QEMU_IO -c 'read -P 0x00 0 128k' "$TEST_IMG" | _filter_qemu_io
echo "# Output of qemu-img map"
$QEMU_IMG map "$TEST_IMG" | _filter_testdir
done
echo
echo "### Test 'qemu-io -c discard' on a QCOW2 image with a backing file"
echo
echo "# Create a backing image and fill it with data"
BACKING_IMG="$TEST_IMG.base"
TEST_IMG="$BACKING_IMG" _make_test_img 128k
$QEMU_IO -c 'write -P 0xff 0 128k' "$BACKING_IMG" | _filter_qemu_io
for qcow2_compat in 0.10 1.1; do
echo "# Create an image with compat=$qcow2_compat and a backing file"
_make_test_img -o "compat=$qcow2_compat" -b "$BACKING_IMG"
echo "# Fill all clusters with data and then discard them"
$QEMU_IO -c 'write -P 0x01 0 128k' "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c 'discard 0 128k' "$TEST_IMG" | _filter_qemu_io
echo "# Read the data from the discarded clusters"
if [ "$qcow2_compat" = "1.1" ]; then
# In qcow2 v3 clusters are zeroed (with QCOW_OFLAG_ZERO)
$QEMU_IO -c 'read -P 0x00 0 128k' "$TEST_IMG" | _filter_qemu_io
else
# In qcow2 v2 if there's a backing image we cannot zero the clusters
# without exposing the backing file data so discard does nothing
$QEMU_IO -c 'read -P 0x01 0 128k' "$TEST_IMG" | _filter_qemu_io
fi
echo "# Output of qemu-img map"
$QEMU_IMG map "$TEST_IMG" | _filter_testdir
done
# success, all done
echo "*** done"
rm -f $seq.full
status=0

View file

@ -0,0 +1,61 @@
QA output created by 290
### Test 'qemu-io -c discard' on a QCOW2 image without a backing file
# Create an image with compat=0.10 without a backing file
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
# Fill all clusters with data and then discard them
wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
discard 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Read the data from the discarded clusters
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Output of qemu-img map
Offset Length Mapped to File
# Create an image with compat=1.1 without a backing file
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
# Fill all clusters with data and then discard them
wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
discard 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Read the data from the discarded clusters
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Output of qemu-img map
Offset Length Mapped to File
### Test 'qemu-io -c discard' on a QCOW2 image with a backing file
# Create a backing image and fill it with data
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=131072
wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Create an image with compat=0.10 and a backing file
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 backing_file=TEST_DIR/t.IMGFMT.base
# Fill all clusters with data and then discard them
wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
discard 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Read the data from the discarded clusters
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Output of qemu-img map
Offset Length Mapped to File
0 0x20000 0x50000 TEST_DIR/t.qcow2
# Create an image with compat=1.1 and a backing file
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 backing_file=TEST_DIR/t.IMGFMT.base
# Fill all clusters with data and then discard them
wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
discard 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Read the data from the discarded clusters
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Output of qemu-img map
Offset Length Mapped to File
*** done

View file

@ -296,3 +296,4 @@
286 rw quick
288 quick
289 rw quick
290 rw auto quick