On 2017-11-01 16:42, Alberto Garcia wrote: > 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 <nasas...@in.ibm.com> > Signed-off-by: Alberto Garcia <be...@igalia.com> > --- > block/qcow2-refcount.c | 7 +++++++ > tests/qemu-iotests/060 | 11 +++++++++++ > tests/qemu-iotests/060.out | 8 ++++++++ > 3 files changed, 26 insertions(+) > > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > index aa3fd6cf17..9059996c4b 100644 > --- a/block/qcow2-refcount.c > +++ b/block/qcow2-refcount.c > @@ -367,6 +367,13 @@ static int alloc_refcount_block(BlockDriverState *bs, > return new_block; > } > > + /* 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. Reviewed-by: Max Reitz <mre...@redhat.com> > + "allocation of refcount block at offset 0"); > + return -EIO; > + } > + > #ifdef DEBUG_ALLOC2 > fprintf(stderr, "qcow2: Allocate refcount block %d for %" PRIx64 > " at %" PRIx64 "\n", > diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 > index 8e95c450eb..dead26aeaf 100755 > --- a/tests/qemu-iotests/060 > +++ b/tests/qemu-iotests/060 > @@ -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 > diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out > index 5ca3af491f..872719009c 100644 > --- a/tests/qemu-iotests/060.out > +++ b/tests/qemu-iotests/060.out > @@ -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 >
signature.asc
Description: OpenPGP digital signature
