Misc qcow2 corruption checks

Each entry in the qcow2 cache contains an offset field indicating the location of the data in the qcow2 image. If the offset is 0 then it means that the entry contains no data and is available to be used when needed.

Because of that it is not possible to store in the cache the first cluster of the qcow2 image (offset = 0). This is not a problem because that cluster always contains the qcow2 header and we're not using this cache for that.

However, if the qcow2 image is corrupted it can happen that we try to allocate a new refcount block at offset 0, triggering this assertion and crashing QEMU:

qcow2_cache_entry_mark_dirty: Assertion `c->entries[i].offset != 0' failed

This patch adds an explicit check for this scenario and a new test case.

This problem was originally reported here:

   https://bugs.launchpad.net/qemu/+bug/1728615

Reported-by R.Nageswara Sastry <[email protected]>

Signed-off-by Alberto Garcia <[email protected]>

---
 block/qcow2-refcount.c     |  7 +++++++
 tests/qemu-iotests/060     | 11 +++++++++++
 tests/qemu-iotests/060.out |  8 ++++++++
 3 files changed, 26 insertions(+)

block/qcow2-refcount.c +0 -0

index aa3fd6cf17..9059996c4b 100644

block/qcow2-refcount.c +4 -0

+                                "allocation of refcount block at offset 0");
+        return -EIO;
+    }
+
 #ifdef DEBUG_ALLOC2
     fprintf(stderr, "qcow2: Allocate refcount block %d for %" PRIx64
         " at %" PRIx64 "\n",

tests/qemu-iotests/060 +11 -0

index 8e95c450eb..dead26aeaf 100755
@@ -242,6 +242,17 @@ poke_file "$TEST_IMG" "$(($l2_offset+8))" "\x80\x00\x00\x00\x00\x06\x2a\x00"
 # Should emit two error messages
 $QEMU_IO -c "discard 0 64k" -c "read 64k 64k" "$TEST_IMG" | _filter_qemu_io
 
+echo
+echo "=== Testing empty refcount table with valid L1 and L2 tables ==="
+echo
+_make_test_img 64M
+$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
+poke_file "$TEST_IMG" "$rt_offset"        "\x00\x00\x00\x00\x00\x00\x00\x00"
+# Since the first data cluster is already allocated this triggers an
+# allocation with an explicit offset (using qcow2_alloc_clusters_at())
+# causing a refcount block to be allocated at offset 0
+$QEMU_IO -c "write 0 128k" "$TEST_IMG" | _filter_qemu_io
+
 # success, all done
 echo "*** done"
 rm -f $seq.full

tests/qemu-iotests/060.out +8 -0

index 5ca3af491f..872719009c 100644
@@ -181,4 +181,12 @@ qcow2: Marking image as corrupt: Cluster allocation offset 0x62a00 unaligned (L2
 discard 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read failed: Input/output error
+
+=== Testing empty refcount table with valid L1 and L2 tables ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 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 allocation of refcount block at offset 0; further corruption events will be suppressed
+write failed: Input/output error
 *** done

Quoted 8 lines

block/qcow2-refcount.c +0 -0

index aa3fd6cf17..9059996c4b 100644
@@ -367,6 +367,13 @@ static int alloc_refcount_block(BlockDriverState *bs,
         return new_block;
     }
 

block/qcow2-refcount.c static int alloc_refcount_block(BlockDriverState *bs, · line 372

Quoted 3 lines

block/qcow2-refcount.c +3 -0

+    /* If we're allocating the block at offset 0 then something is wrong */
+    if (new_block == 0) {
+        qcow2_signal_corruption(bs, true, -1, -1, "Preventing invalid "

Technically, the offset is 0 and the size is s->cluster_size, but I won't insist.

If the refcount data is corrupted then we can end up trying to allocate a new L2 table at offset 0 in the image, triggering an assertion in the qcow2 cache that would crash QEMU:

qcow2_cache_entry_mark_dirty: Assertion `c->entries[i].offset != 0' failed

This patch adds an explicit check for this scenario and a new test case.

Signed-off-by Alberto Garcia <[email protected]>

---
 block/qcow2-cluster.c      | 7 +++++++
 tests/qemu-iotests/060     | 7 +++++++
 tests/qemu-iotests/060.out | 6 ++++++
 3 files changed, 20 insertions(+)

block/qcow2-cluster.c +0 -0

index fb10e26068..540af4d19d 100644

block/qcow2-cluster.c +2 -0

+    }
+
     ret = qcow2_cache_flush(bs, s->refcount_block_cache);
     if (ret < 0) {
         goto fail;

tests/qemu-iotests/060 +7 -0

index dead26aeaf..40f85cc216 100755
@@ -253,6 +253,13 @@ poke_file "$TEST_IMG" "$rt_offset"        "\x00\x00\x00\x00\x00\x00\x00\x00"
 # causing a refcount block to be allocated at offset 0
 $QEMU_IO -c "write 0 128k" "$TEST_IMG" | _filter_qemu_io
 
+echo
+echo "=== Testing empty refcount block ==="
+echo
+_make_test_img 64M
+poke_file "$TEST_IMG" "$rb_offset"        "\x00\x00\x00\x00\x00\x00\x00\x00"
+$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
+
 # success, all done
 echo "*** done"
 rm -f $seq.full

tests/qemu-iotests/060.out +6 -0

index 872719009c..5b8b518486 100644
@@ -189,4 +189,10 @@ wrote 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 allocation of refcount block at offset 0; further corruption events will be suppressed
 write failed: Input/output error
+
+=== Testing empty refcount block ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qcow2: Marking image as corrupt: Preventing invalid allocation of L2 table at offset 0; further corruption events will be suppressed
+write failed: Input/output error
 *** done

block/qcow2-cluster.c static int l2_allocate(BlockDriverState *bs, int l1_index, uint64_t **table) · line 285

Quoted 5 lines

block/qcow2-cluster.c +5 -0

+    /* If we're allocating the table at offset 0 then something is wrong */
+    if (l2_offset == 0) {
+        qcow2_signal_corruption(bs, true, -1, -1, "Preventing invalid "
+                                "allocation of L2 table at offset 0");
+        return -EIO;

The other error paths in this function use a goto fail. It doesn't make a functional difference, but not doing it here looks a bit weird.

qcow2_do_open() is checking that header.refcount_table_clusters is not too large, but it doesn't check that it's greater than zero. Apart from the fact that an image like that is obviously corrupted, trying to use it crashes QEMU since we end up with a null s->refcount_table after qcow2_refcount_init().

These images can however be repaired, so allow opening them if the BDRV_O_CHECK flag is set.

Signed-off-by Alberto Garcia <[email protected]>

---
 block/qcow2.c              | 6 ++++++
 tests/qemu-iotests/060     | 7 +++++++
 tests/qemu-iotests/060.out | 5 +++++
 3 files changed, 18 insertions(+)

This patch adds a simple iotests in which we try to write to an image with an empty refcount table (i.e. with all entries set to 0).

This scenario was already handled by the existing consistency checks, but we add an explicit test case for completeness.

Signed-off-by Alberto Garcia <[email protected]>

---
 tests/qemu-iotests/060     | 7 +++++++
 tests/qemu-iotests/060.out | 6 ++++++
 2 files changed, 13 insertions(+)

Misc qcow2 corruption checks

This series contains a few checks that prevent QEMU from crashing under some scenarios with corrupted qcow2 images.

The first patch solves the crash reported here:

https://bugs.launchpad.net/qemu/+bug/1728615

And the others solve similar crashes that I detected in the process of fixing this one.

Regards,

Berto

There are two more cases which might need a check that the return value of an allocation function isn't 0:

The first is qcow2_alloc_bytes() which has an assert(offset) after potentially setting offset = new_cluster (with new_cluster being the return value of alloc_clusters_noref()).

The second is qcow2_crypto_hdr_init_func() which is simply missing a pre-write overlap check.