qcow2: Fix backing file name length check

len could become negative and would pass the check then. Nothing bad
happened because bdrv_pread() happens to return an error for negative
length values, but make variables for sizes unsigned anyway.

This patch also changes the behaviour to error out on invalid lengths
instead of silently truncating it to 1023.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
(cherry picked from commit 6d33e8e7dc9d40ea105feed4b39caa3e641569e8)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

diff --git a/block/qcow2.c b/block/qcow2.c
index f1f8c9c3407bda26536d3e9e8cbd91d2c5eb6fc4..3e620f2127465ee234b11ce663e4406c89624509 100644 (file)
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -448,7 +448,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
                       Error **errp)
 {
     BDRVQcowState *s = bs->opaque;
-    int len, i, ret = 0;
+    unsigned int len, i;
+    int ret = 0;
     QCowHeader header;
     QemuOpts *opts;
     Error *local_err = NULL;
@@ -723,8 +724,10 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     /* read the backing file name */
     if (header.backing_file_offset != 0) {
         len = header.backing_file_size;
-        if (len > 1023) {
-            len = 1023;
+        if (len > MIN(1023, s->cluster_size - header.backing_file_offset)) {
+            error_setg(errp, "Backing file name too long");
+            ret = -EINVAL;
+            goto fail;
         }
         ret = bdrv_pread(bs->file, header.backing_file_offset,
                          bs->backing_file, len);

diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 7255b6cca635a9c8ab975176aa497e911afaf585..f3091a9377db61bc4b92971ca88183748973061e 100755 (executable)
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -45,6 +45,7 @@ _supported_os Linux
 header_size=104
 
 offset_backing_file_offset=8
+offset_backing_file_size=16
 offset_l1_size=36
 offset_l1_table_offset=40
 offset_refcount_table_offset=48
@@ -135,6 +136,13 @@ poke_file "$TEST_IMG" "$offset_l1_table_offset" "\x12\x34\x56\x78\x90\xab\xcd\xe
 poke_file "$TEST_IMG" "$offset_l1_size" "\x00\x00\x00\x01"
 { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Invalid backing file size =="
+_make_test_img 64M
+poke_file "$TEST_IMG" "$offset_backing_file_offset" "\x00\x00\x00\x00\x00\x00\x10\x00"
+poke_file "$TEST_IMG" "$offset_backing_file_size" "\xff\xff\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
 # success, all done
 echo "*** done"
 rm -f $seq.full

diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out
index 4ec2545051d54cc62b4d75e4a5e753002f16a5c7..8103211064de7692692fdfec86b2ef9f2468ad79 100644 (file)
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -58,4 +58,9 @@ qemu-io: can't open device TEST_DIR/t.qcow2: Invalid L1 table offset
 no file open, try 'help open'
 qemu-io: can't open device TEST_DIR/t.qcow2: Invalid L1 table offset
 no file open, try 'help open'
+
+== Invalid backing file size ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-io: can't open device TEST_DIR/t.qcow2: Backing file name too long
+no file open, try 'help open'
 *** done