[PATCH v3] qcow2: Avoid integer wraparound in qcow2_co_truncate()

* [PATCH v3] qcow2: Avoid integer wraparound in qcow2_co_truncate()
@ 2020-05-04 15:52 Alberto Garcia
  2020-05-04 16:01 ` Eric Blake
  2020-05-05  8:33 ` no-reply
  0 siblings, 2 replies; 8+ messages in thread
From: Alberto Garcia @ 2020-05-04 15:52 UTC (permalink / raw)
  To: qemu-devel, Kevin Wolf; +Cc: Alberto Garcia, qemu-block, Max Reitz

After commit f01643fb8b47e8a70c04bbf45e0f12a9e5bc54de when an image is
extended and BDRV_REQ_ZERO_WRITE is set then the new clusters are
zeroized.

The code however does not detect correctly situations when the old and
the new end of the image are within the same cluster. The problem can
be reproduced with these steps:

   qemu-img create -f qcow2 backing.qcow2 1M
   qemu-img create -f qcow2 -F qcow2 -b backing.qcow2 top.qcow2
   qemu-img resize --shrink top.qcow2 520k
   qemu-img resize top.qcow2 567k

In the last step offset - zero_start causes an integer wraparound.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
v3:
- Add test case [Kevin]
- Move MIN(zero_start, offset) to the block that writes zeroes [Kevin]

v2:
- Don't call qcow2_cluster_zeroize() if offset == zero_start

 block/qcow2.c              | 12 ++++---
 tests/qemu-iotests/292     | 73 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/292.out | 24 +++++++++++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 105 insertions(+), 5 deletions(-)
 create mode 100755 tests/qemu-iotests/292
 create mode 100644 tests/qemu-iotests/292.out

diff --git a/block/qcow2.c b/block/qcow2.c
index 2ba0b17c39..8c97b06783 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4239,15 +4239,17 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
          * requires a cluster-aligned start. The end may be unaligned if it is
          * at the end of the image (which it is here).
          */
-        ret = qcow2_cluster_zeroize(bs, zero_start, offset - zero_start, 0);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Failed to zero out new clusters");
-            goto fail;
+        if (offset > zero_start) {
+            ret = qcow2_cluster_zeroize(bs, zero_start, offset - zero_start, 0);
+            if (ret < 0) {
+                error_setg_errno(errp, -ret, "Failed to zero out new clusters");
+                goto fail;
+            }
         }
 
         /* Write explicit zeros for the unaligned head */
         if (zero_start > old_length) {
-            uint64_t len = zero_start - old_length;
+            uint64_t len = MIN(zero_start, offset) - old_length;
             uint8_t *buf = qemu_blockalign0(bs, len);
             QEMUIOVector qiov;
             qemu_iovec_init_buf(&qiov, buf, len);
diff --git a/tests/qemu-iotests/292 b/tests/qemu-iotests/292
new file mode 100755
index 0000000000..a2de27cca4
--- /dev/null
+++ b/tests/qemu-iotests/292
@@ -0,0 +1,73 @@
+#!/usr/bin/env bash
+#
+# Test resizing a qcow2 image with a backing file
+#
+# 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
+
+echo '### Create the backing image'
+BACKING_IMG="$TEST_IMG.base"
+TEST_IMG="$BACKING_IMG" _make_test_img 1M
+
+echo '### Fill the backing image with data (0x11)'
+$QEMU_IO -c 'write -P 0x11 0 1M' "$BACKING_IMG" | _filter_qemu_io
+
+echo '### Create the top image'
+_make_test_img -F "$IMGFMT" -b "$BACKING_IMG"
+
+echo '### Fill the top image with data (0x22)'
+$QEMU_IO -c 'write -P 0x22 0 1M' "$TEST_IMG" | _filter_qemu_io
+
+# Both offsets are part of the same cluster.
+echo '### Shrink the image to 520k'
+$QEMU_IMG resize --shrink "$TEST_IMG" 520k
+echo '### Grow the image to 567k'
+$QEMU_IMG resize "$TEST_IMG" 567k
+
+echo '### Check that the tail of the image reads as zeroes'
+$QEMU_IO -c 'read -P 0x22    0 520k' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0x00 520k  47k' "$TEST_IMG" | _filter_qemu_io
+
+echo '### Show output of qemu-img map'
+$QEMU_IMG map "$TEST_IMG" | _filter_testdir
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/292.out b/tests/qemu-iotests/292.out
new file mode 100644
index 0000000000..807e0530c3
--- /dev/null
+++ b/tests/qemu-iotests/292.out
@@ -0,0 +1,24 @@
+QA output created by 292
+### Create the backing image
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+### Fill the backing image with data (0x11)
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+### Create the top image
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
+### Fill the top image with data (0x22)
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+### Shrink the image to 520k
+Image resized.
+### Grow the image to 567k
+Image resized.
+### Check that the tail of the image reads as zeroes
+read 532480/532480 bytes at offset 0
+520 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 48128/48128 bytes at offset 532480
+47 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+### Show output of qemu-img map
+Offset          Length          Mapped to       File
+0               0x8dc00         0x50000         TEST_DIR/t.qcow2
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1710470e70..fe649c5b73 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -298,3 +298,4 @@
 288 quick
 289 rw quick
 290 rw auto quick
+292 rw auto quick
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread