On 2017-11-03 15:18, Alberto Garcia wrote:
> If the refcount data is corrupted then we can end up trying to
> allocate a new compressed cluster at offset 0 in the image, triggering
> an assertion in qcow2_alloc_bytes() that would crash QEMU:
>
> qcow2_alloc_bytes: Assertion `offset' failed.
>
> This patch adds an explicit check for this scenario and a new test
> case.
>
> Signed-off-by: Alberto Garcia <be...@igalia.com>
> ---
> block/qcow2-refcount.c | 8 +++++++-
> tests/qemu-iotests/060 | 10 ++++++++++
> tests/qemu-iotests/060.out | 8 ++++++++
> 3 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 9059996c4b..7eaac11429 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1082,6 +1082,13 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int
> size)
> return new_cluster;
> }
>
> + if (new_cluster == 0) {
> + qcow2_signal_corruption(bs, true, -1, -1, "Preventing
> invalid "
> + "allocation of compressed cluster "
> + "at offset 0");
> + return -EIO;
> + }
> +
> if (!offset || ROUND_UP(offset, s->cluster_size) != new_cluster)
> {
> offset = new_cluster;
> free_in_cluster = s->cluster_size;
> @@ -1090,7 +1097,6 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int
> size)
> }
> }
>
> - assert(offset);
I don't think this assert() was meant as a protection against offset
being 0. :-)
Reviewed-by: Max Reitz <mre...@redhat.com>
if the assert() stays.
> ret = update_refcount(bs, offset, size, 1, false,
> QCOW2_DISCARD_NEVER);
> if (ret < 0) {
> offset = 0;
> diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
> index 40f85cc216..c3bce27b33 100755
> --- a/tests/qemu-iotests/060
> +++ b/tests/qemu-iotests/060
> @@ -260,6 +260,16 @@ _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
>
> +echo
> +echo "=== Testing empty refcount block with compressed write ==="
> +echo
> +_make_test_img 64M
> +$QEMU_IO -c "write 64k 64k" "$TEST_IMG" | _filter_qemu_io
> +poke_file "$TEST_IMG" "$rb_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
> +# The previous write already allocated an L2 table, so now this new
> +# write will try to allocate a compressed data cluster at offset 0.
> +$QEMU_IO -c "write -c 0k 64k" "$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 5b8b518486..cf8790ff57 100644
> --- a/tests/qemu-iotests/060.out
> +++ b/tests/qemu-iotests/060.out
> @@ -195,4 +195,12 @@ write failed: Input/output error
> 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
> +
> +=== Testing empty refcount block with compressed write ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
> +wrote 65536/65536 bytes at offset 65536
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +qcow2: Marking image as corrupt: Preventing invalid allocation of compressed
> cluster at offset 0; further corruption events will be suppressed
> +write failed: Input/output error
> *** done
>
signature.asc
Description: OpenPGP digital signature
