| Message ID | f9750f50c80359babba11062e88f5075a47e8e16.1509718618.git.berto@igalia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Misc qcow2 corruption checks | expand |
Am 03.11.2017 um 15:18 hat Alberto Garcia geschrieben: > 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 <berto@igalia.com> > Reviewed-by: Max Reitz <mreitz@redhat.com> > --- a/tests/qemu-iotests/060 > +++ b/tests/qemu-iotests/060 > @@ -270,6 +270,13 @@ poke_file "$TEST_IMG" "$rb_offset" "\x00\x00\x00\x00\x00\x00\x00\x00" > # write will try to allocate a compressed data cluster at offset 0. > $QEMU_IO -c "write -c 0k 64k" "$TEST_IMG" | _filter_qemu_io > > +echo > +echo "=== Testing zero refcount table size ===" > +echo > +_make_test_img 64M > +poke_file "$TEST_IMG" "56" "\x00\x00\x00\x00" > +$QEMU_IO -c "write 0 64k" "$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt In the commit message, you claim that the image can be repaired. Would it be worth actually testing the repair here? Kevin
On Tue 07 Nov 2017 05:43:49 PM CET, Kevin Wolf wrote: >> +echo >> +echo "=== Testing zero refcount table size ===" >> +echo >> +_make_test_img 64M >> +poke_file "$TEST_IMG" "56" "\x00\x00\x00\x00" >> +$QEMU_IO -c "write 0 64k" "$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt > > In the commit message, you claim that the image can be repaired. Would > it be worth actually testing the repair here? Good idea. Max already merged this series in his branch, so I'll just send a follow-up patch with this. Berto
diff --git a/block/qcow2.c b/block/qcow2.c index 92cb9f9bfa..defc1fe49f 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1280,6 +1280,12 @@ static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } + if (header.refcount_table_clusters == 0 && !(flags & BDRV_O_CHECK)) { + error_setg(errp, "Image does not contain a reference count table"); + ret = -EINVAL; + goto fail; + } + ret = validate_table_offset(bs, s->refcount_table_offset, s->refcount_table_size, sizeof(uint64_t)); if (ret < 0) { diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 index c3bce27b33..656af50883 100755 --- a/tests/qemu-iotests/060 +++ b/tests/qemu-iotests/060 @@ -270,6 +270,13 @@ poke_file "$TEST_IMG" "$rb_offset" "\x00\x00\x00\x00\x00\x00\x00\x00" # write will try to allocate a compressed data cluster at offset 0. $QEMU_IO -c "write -c 0k 64k" "$TEST_IMG" | _filter_qemu_io +echo +echo "=== Testing zero refcount table size ===" +echo +_make_test_img 64M +poke_file "$TEST_IMG" "56" "\x00\x00\x00\x00" +$QEMU_IO -c "write 0 64k" "$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out index cf8790ff57..58456e8487 100644 --- a/tests/qemu-iotests/060.out +++ b/tests/qemu-iotests/060.out @@ -203,4 +203,9 @@ 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 + +=== Testing zero refcount table size === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +can't open device TEST_DIR/t.IMGFMT: Image does not contain a reference count table *** done