parallels_open() copied nb_sectors, tracks, and bat_entries from the image header without checking that the BAT actually covers the advertised virtual disk size. An image whose header claims more sectors than its BAT covers passes the generic block-layer bounds check on open. A write into the gap between BAT coverage and the advertised size then reaches allocate_clusters(), whose internal assert(idx < s->bat_size && idx + to_allocate <= s->bat_size) aborts the process instead of returning a normal I/O error.
Reject such images at open time by requiring bat_size * tracks >= total_sectors, matching the invariant that allocate_clusters() already assumes. Reported-by: Feifan Qian <[email protected]> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3804 Signed-off-by: Denis V. Lunev <[email protected]> CC: Thomas Huth <[email protected]> CC: Stefan Hajnoczi <[email protected]> --- block/parallels.c | 6 ++++++ tests/qemu-iotests/tests/parallels-checks | 21 +++++++++++++++++++ tests/qemu-iotests/tests/parallels-checks.out | 7 +++++++ 3 files changed, 34 insertions(+) diff --git a/block/parallels.c b/block/parallels.c index 8a7e8b4aba..93b5fa9dcd 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -1328,6 +1328,12 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, return -EFBIG; } + if ((uint64_t)s->bat_size * s->tracks < bs->total_sectors) { + error_setg(errp, "Invalid image: Catalog size too small for " + "advertised disk size"); + return -EINVAL; + } + size = bat_entry_off(s->bat_size); s->header_size = ROUND_UP(size, bdrv_opt_mem_align(bs->file->bs)); s->header = qemu_try_blockalign(bs->file->bs, s->header_size); diff --git a/tests/qemu-iotests/tests/parallels-checks b/tests/qemu-iotests/tests/parallels-checks index eef1c86809..66199fff1a 100755 --- a/tests/qemu-iotests/tests/parallels-checks +++ b/tests/qemu-iotests/tests/parallels-checks @@ -225,6 +225,27 @@ echo "== an unallocated cluster still reads as zeroes ==" # Clear image _make_test_img $SIZE +echo "== TEST OVERSIZED VIRTUAL DISK CHECK ==" + +BAT_ENTRIES_OFFSET=32 +NB_SECTORS_OFFSET=36 + +TRACKS=$(peek_file_le $TEST_IMG $CLUSTER_SIZE_OFFSET 4) +BAT_ENTRIES=$(peek_file_le $TEST_IMG $BAT_ENTRIES_OFFSET 4) +COVERED_SECTORS=$((BAT_ENTRIES * TRACKS)) + +echo "== advertise one more cluster than the BAT covers ==" +poke_file_le "$TEST_IMG" $NB_SECTORS_OFFSET 8 $((COVERED_SECTORS + TRACKS)) + +echo "== open must fail cleanly instead of aborting ==" +{ $QEMU_IMG info "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir + +echo "== write into the uncovered range must fail cleanly too ==" +{ $QEMU_IO -c "write -P 0x41 $((COVERED_SECTORS * 512)) $CLUSTER_SIZE" "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir + +# Clear image +_make_test_img $SIZE + echo "== TEST BAT ENTRY POINTING OUTSIDE IMAGE ==" echo "== corrupt image: point first cluster far outside the file ==" diff --git a/tests/qemu-iotests/tests/parallels-checks.out b/tests/qemu-iotests/tests/parallels-checks.out index 86d5b6ac2d..7b73498271 100644 --- a/tests/qemu-iotests/tests/parallels-checks.out +++ b/tests/qemu-iotests/tests/parallels-checks.out @@ -147,6 +147,13 @@ Child node '/file': read 1048576/1048576 bytes at offset 0 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304 +== TEST OVERSIZED VIRTUAL DISK CHECK == +== advertise one more cluster than the BAT covers == +== open must fail cleanly instead of aborting == +qemu-img: Could not open 'TEST_DIR/t.parallels': Invalid image: Catalog size too small for advertised disk size +== write into the uncovered range must fail cleanly too == +qemu-io: can't open device TEST_DIR/t.parallels: Invalid image: Catalog size too small for advertised disk size +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304 == TEST BAT ENTRY POINTING OUTSIDE IMAGE == == corrupt image: point first cluster far outside the file == == read-only read must return zeroes, not an I/O error == -- 2.53.0
