parallels_co_create() computed the BAT entry count directly into a uint32_t, wrapping silently to zero at exactly 2^32 entries and writing out a header whose BAT no longer matches its advertised size. Compute it in an int64_t first and reject it once it no longer fits, matching the cap parallels_open() already enforces. Also reject cluster-size 0, and clamp header.cylinders instead of letting it truncate the same way.
Signed-off-by: Denis V. Lunev <[email protected]> CC: Thomas Huth <[email protected]> CC: Stefan Hajnoczi <[email protected]> --- block/parallels.c | 23 +++++++++++++++++------ tests/qemu-iotests/212 | 8 ++++++-- tests/qemu-iotests/212.out | 10 ++++++++-- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index 0f655b58d5..e3d26a6650 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -999,7 +999,8 @@ parallels_co_create(BlockdevCreateOptions* opts, Error **errp) BlockdevCreateOptionsParallels *parallels_opts; BlockDriverState *bs; BlockBackend *blk; - int64_t total_size, cl_size; + int64_t total_size, cl_size, bat_count; + uint64_t cylinders; uint32_t bat_entries, bat_sectors; ParallelsHeader header; uint8_t tmp[BDRV_SECTOR_SIZE]; @@ -1017,16 +1018,22 @@ parallels_co_create(BlockdevCreateOptions* opts, Error **errp) cl_size = DEFAULT_CLUSTER_SIZE; } - /* XXX What is the real limit here? This is an insanely large maximum. */ + /* Bounds cl_size so the multiplication below can't overflow int64_t. */ if (cl_size >= INT64_MAX / MAX_PARALLELS_IMAGE_FACTOR) { error_setg(errp, "Cluster size is too large"); return -EINVAL; } - if (total_size >= MAX_PARALLELS_IMAGE_FACTOR * cl_size) { + if (cl_size <= 0 || total_size >= MAX_PARALLELS_IMAGE_FACTOR * cl_size) { error_setg(errp, "Image size is too large for this cluster size"); return -E2BIG; } + bat_count = DIV_ROUND_UP(total_size, cl_size); + if (bat_count > INT_MAX / (int64_t)sizeof(uint32_t)) { + error_setg(errp, "Catalog too large"); + return -EFBIG; + } + if (!QEMU_IS_ALIGNED(total_size, BDRV_SECTOR_SIZE)) { error_setg(errp, "Image size must be a multiple of 512 bytes"); return -EINVAL; @@ -1052,7 +1059,7 @@ parallels_co_create(BlockdevCreateOptions* opts, Error **errp) blk_set_allow_write_beyond_eof(blk, true); /* Create image format */ - bat_entries = DIV_ROUND_UP(total_size, cl_size); + bat_entries = bat_count; bat_sectors = DIV_ROUND_UP(bat_entry_off(bat_entries), cl_size); bat_sectors = (bat_sectors * cl_size) >> BDRV_SECTOR_BITS; @@ -1061,8 +1068,12 @@ parallels_co_create(BlockdevCreateOptions* opts, Error **errp) header.version = cpu_to_le32(HEADER_VERSION); /* don't care much about geometry, it is not used on image level */ header.heads = cpu_to_le32(HEADS_NUMBER); - header.cylinders = cpu_to_le32(total_size / BDRV_SECTOR_SIZE - / HEADS_NUMBER / SEC_IN_CYL); + cylinders = total_size / BDRV_SECTOR_SIZE / HEADS_NUMBER / SEC_IN_CYL; + /* Write only by spec, do not care */ + if (cylinders >= UINT32_MAX) { + cylinders = UINT32_MAX; + } + header.cylinders = cpu_to_le32(cylinders); header.tracks = cpu_to_le32(cl_size >> BDRV_SECTOR_BITS); header.bat_entries = cpu_to_le32(bat_entries); header.nb_sectors = cpu_to_le64(DIV_ROUND_UP(total_size, BDRV_SECTOR_SIZE)); diff --git a/tests/qemu-iotests/212 b/tests/qemu-iotests/212 index d4af0c4ac8..4ca6149b6b 100755 --- a/tests/qemu-iotests/212 +++ b/tests/qemu-iotests/212 @@ -133,13 +133,15 @@ with iotests.FilePath('t.parallels') as disk_path, \ # # Maximum size # + # Largest catalog parallels_open() can address. + # iotests.log("=== Maximum size ===") iotests.log("") vm.launch() vm.blockdev_create({ 'driver': imgfmt, 'file': 'node0', - 'size': 4503599627369984}) + 'size': 562949952372736}) vm.shutdown() iotests.img_info_log(disk_path) @@ -158,13 +160,15 @@ with iotests.FilePath('t.parallels') as disk_path, \ # 4. 2^63 - 512 (generally valid, but with the image header the file will # exceed 63 bits) # 5. 2^52 (512 bytes more than maximum image size) + # 6. 2^52 - 512 (wraps bat_entries to 0 at the default 1 MiB cluster size) iotests.log("=== Invalid sizes ===") iotests.log("") vm.launch() for size in [ 1234, 18446744073709551104, 9223372036854775808, - 9223372036854775296, 4503599627370497 ]: + 9223372036854775296, 4503599627370497, + 4503599627369984 ]: vm.blockdev_create({ 'driver': imgfmt, 'file': 'node0', 'size': size }) diff --git a/tests/qemu-iotests/212.out b/tests/qemu-iotests/212.out index 8102033488..d59f8ed44b 100644 --- a/tests/qemu-iotests/212.out +++ b/tests/qemu-iotests/212.out @@ -69,14 +69,14 @@ virtual size: 0 B (0 bytes) === Maximum size === -{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "parallels", "file": "node0", "size": 4503599627369984}}} +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "parallels", "file": "node0", "size": 562949952372736}}} {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job0"}} {"return": {}} image: TEST_IMG file format: IMGFMT -virtual size: 4 PiB (4503599627369984 bytes) +virtual size: 512 TiB (562949952372736 bytes) === Invalid sizes === @@ -110,6 +110,12 @@ Job failed: Image size is too large for this cluster size {"execute": "job-dismiss", "arguments": {"id": "job0"}} {"return": {}} +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "parallels", "file": "node0", "size": 4503599627369984}}} +{"return": {}} +Job failed: Catalog too large +{"execute": "job-dismiss", "arguments": {"id": "job0"}} +{"return": {}} + === Invalid cluster size === {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"cluster-size": 1234, "driver": "parallels", "file": "node0", "size": 67108864}}} -- 2.53.0
