[PULL v2 0/9] parallels: fix reachable assertion and related bounds-checking gaps

[PULL v2 0/9] parallels: fix reachable assertion and related bounds-checking gaps

[Denis V. Lunev]

The following changes since commit 6333226c2abb72f31648c251624c56e70993d625:

  Merge tag 'pull-9p-20260725' of https://github.com/cschoenebeck/qemu into staging (2026-07-26 08:30:07 -0400)

are available in the Git repository at:

  https://gitlab.com/dlunev/qemu.git tags/pull-parallels-2026-07-28

for you to fetch changes up to 352587cbe2d4cde1cece16e2f6a9e1e4190a03e6:

  MAINTAINERS: update parallels tree location (2026-07-28 16:55:00 +0200)

----------------------------------------------------------------
Parallels block driver patches

- fix a reachable assert()/process abort on a crafted image with an
  inconsistent BAT vs. advertised disk size (GitLab #3804)
- fix related integer overflows capping the format's usable catalog
  size below its documented maximum
- reject BAT entries pointing outside the data area in either
  direction
- harden the dirty-bitmap extension loader against a reachable
  abort and an unsafe allocator

Changes since v1:

- filter out format specific info in patches 2,8

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Thomas Huth <thuth@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>

----------------------------------------------------------------
Denis V. Lunev (9):
  parallels: fix integer overflow in header size calculation
  parallels: read header/BAT table in bounded chunks
  parallels: fix bat_entries overflow in image creation
  parallels: reject BAT entries pointing outside backed storage
  parallels: validate bitmap L1 table size before allocating it
  parallels: skip loading a genuinely empty bitmap L1 table
  parallels: avoid fatal abort on large bitmap L1 table
  parallels: validate BAT capacity against advertised disk size
  MAINTAINERS: update parallels tree location

 MAINTAINERS                                   |   2 +-
 block/parallels-ext.c                         |  52 +++++----
 block/parallels.c                             |  74 ++++++++++---
 tests/qemu-iotests/212                        |   8 +-
 tests/qemu-iotests/212.out                    |  10 +-
 tests/qemu-iotests/tests/parallels-checks     | 102 ++++++++++++++++++
 tests/qemu-iotests/tests/parallels-checks.out |  53 +++++++++
 7 files changed, 264 insertions(+), 37 deletions(-)

-- 
2.53.0

[PULL v2 1/9] parallels: fix integer overflow in header size calculation

[Denis V. Lunev]

parallels_open() caches bat_entry_off(s->bat_size) - a uint32_t -
in a plain int before it feeds into s->header_size. Near the
"Catalog too large" bound the value exceeds INT_MAX and overflows
on assignment.

Match the cached value's type to bat_entry_off()'s return type.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Thomas Huth <thuth@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/parallels.c b/block/parallels.c
index 7a90fb5220..59f00c64a6 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1240,7 +1240,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
 {
     BDRVParallelsState *s = bs->opaque;
     ParallelsHeader ph;
-    int ret, size, i;
+    int ret, i;
+    uint32_t size;
     int64_t file_nb_sectors, sector;
     uint32_t data_start;
     bool need_check = false;
-- 
2.53.0

[PULL v2 2/9] parallels: read header/BAT table in bounded chunks

[Denis V. Lunev]

parallels_open() read the whole header+BAT table with a single
bdrv_pread() call sized s->header_size. For an image whose catalog
approaches the "Catalog too large" bound (INT_MAX / sizeof(uint32_t)
entries), that size approaches BDRV_REQUEST_MAX_BYTES, and the block
layer legitimately refuses a single request that large, so the image
failed to open with a generic I/O error even though the catalog size
itself is within the format's documented limit.

Read the header and BAT table in fixed-size chunks instead, so the
maximum catalog size parallels_open() can actually address matches
the bound it already enforces, independent of the file's block-layer
alignment requirements.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Thomas Huth <thuth@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c                             | 17 ++++++++++----
 tests/qemu-iotests/tests/parallels-checks     | 23 +++++++++++++++++++
 tests/qemu-iotests/tests/parallels-checks.out | 11 +++++++++
 3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 59f00c64a6..0f655b58d5 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -52,6 +52,7 @@
 #define HEADER_VERSION 2
 #define HEADER_INUSE_MAGIC  (0x746F6E59)
 #define MAX_PARALLELS_IMAGE_FACTOR (1ull << 32)
+#define PARALLELS_HEADER_READ_CHUNK (64 * 1024 * 1024)
 
 static QEnumLookup prealloc_mode_lookup = {
     .array = (const char *const[]) {
@@ -1241,7 +1242,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     BDRVParallelsState *s = bs->opaque;
     ParallelsHeader ph;
     int ret, i;
-    uint32_t size;
+    uint32_t size, header_off;
     int64_t file_nb_sectors, sector;
     uint32_t data_start;
     bool need_check = false;
@@ -1311,9 +1312,17 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         return -ENOMEM;
     }
 
-    ret = bdrv_pread(bs->file, 0, s->header_size, s->header, 0);
-    if (ret < 0) {
-        goto fail;
+    /* A single request s->header_size large exceeds BDRV_REQUEST_MAX_BYTES. */
+    for (header_off = 0; header_off < s->header_size;
+         header_off += PARALLELS_HEADER_READ_CHUNK) {
+        uint32_t chunk = MIN(s->header_size - header_off,
+                             PARALLELS_HEADER_READ_CHUNK);
+
+        ret = bdrv_pread(bs->file, header_off, chunk,
+                         (uint8_t *)s->header + header_off, 0);
+        if (ret < 0) {
+            goto fail;
+        }
     }
     s->bat_bitmap = (uint32_t *)(s->header + 1);
 
diff --git a/tests/qemu-iotests/tests/parallels-checks b/tests/qemu-iotests/tests/parallels-checks
index b281246a42..9535024885 100755
--- a/tests/qemu-iotests/tests/parallels-checks
+++ b/tests/qemu-iotests/tests/parallels-checks
@@ -44,6 +44,7 @@ _supported_os Linux
 SIZE=$((4 * 1024 * 1024))
 IMGFMT=parallels
 CLUSTER_SIZE_OFFSET=28
+BAT_ENTRIES_OFFSET=32
 DATA_OFF_OFFSET=48
 BAT_OFFSET=64
 
@@ -199,6 +200,28 @@ _check_test_img -r all
 echo "== check first cluster =="
 { $QEMU_IO -r -c "read -P 0x55 0 $CLUSTER_SIZE" "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+# Clear image
+_make_test_img $SIZE
+
+echo "== TEST HUGE BAT TABLE OPEN =="
+
+# Overflows a single read request, but stays under parallels_open()'s
+# own catalog-size cap.
+BAT_ENTRIES=536870896
+HEADER_SIZE=$((64 + 4 * BAT_ENTRIES))
+
+echo "== advertise a BAT table larger than BDRV_REQUEST_MAX_BYTES =="
+poke_file "$TEST_IMG" "$BAT_ENTRIES_OFFSET" "\xf0\xff\xff\x1f"
+
+echo "== grow the file to match, without writing real data =="
+truncate -s $HEADER_SIZE "$TEST_IMG"
+
+echo "== open must succeed: the header/BAT read is chunked =="
+_img_info
+
+echo "== an unallocated cluster still reads as zeroes =="
+{ $QEMU_IO -r -c "read -P 0x00 0 $CLUSTER_SIZE" "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/tests/parallels-checks.out b/tests/qemu-iotests/tests/parallels-checks.out
index 9793423111..6fb2014e8e 100644
--- a/tests/qemu-iotests/tests/parallels-checks.out
+++ b/tests/qemu-iotests/tests/parallels-checks.out
@@ -129,4 +129,15 @@ No errors were found on the image.
 == check first cluster ==
 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 HUGE BAT TABLE OPEN ==
+== advertise a BAT table larger than BDRV_REQUEST_MAX_BYTES ==
+== grow the file to match, without writing real data ==
+== open must succeed: the header/BAT read is chunked ==
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 4 MiB (4194304 bytes)
+== an unallocated cluster still reads as zeroes ==
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 *** done
-- 
2.53.0

[PULL v2 3/9] parallels: fix bat_entries overflow in image creation

[Denis V. Lunev]

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 <den@openvz.org>
CC: Thomas Huth <thuth@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 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

[PULL v2 4/9] parallels: reject BAT entries pointing outside backed storage

[Denis V. Lunev]

parallels_open()'s BAT scan and parallels_check_outside_image() only
checked entries against the file's upper end, matching just half of
what docs/interop/parallels.rst requires: an entry's offset must be
both >= data_start and < the file size. An entry below data_start
resolves into the header/BAT region itself, corrupting metadata on
write or losing the write silently on a partial overlap, and neither
qemu-img check nor the open-time scan ever caught it.

Check both bounds everywhere a BAT entry is resolved to a host
offset: seek_to_sector(), the open-time scan (without letting a bad
entry inflate data_end), and parallels_check_outside_image().

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Thomas Huth <thuth@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c                             | 27 +++++++--
 tests/qemu-iotests/tests/parallels-checks     | 58 +++++++++++++++++++
 tests/qemu-iotests/tests/parallels-checks.out | 35 +++++++++++
 3 files changed, 116 insertions(+), 4 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index e3d26a6650..8a7e8b4aba 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -119,6 +119,7 @@ static uint32_t bat_entry_off(uint32_t idx)
 static int64_t seek_to_sector(BDRVParallelsState *s, int64_t sector_num)
 {
     uint32_t index, offset;
+    int64_t cluster_off;
 
     index = sector_num / s->tracks;
     offset = sector_num % s->tracks;
@@ -127,7 +128,14 @@ static int64_t seek_to_sector(BDRVParallelsState *s, int64_t sector_num)
     if ((index >= s->bat_size) || (s->bat_bitmap[index] == 0)) {
         return -1;
     }
-    return bat2sect(s, index) + offset;
+
+    cluster_off = bat2sect(s, index);
+    if (cluster_off < s->data_start || cluster_off + s->tracks > s->data_end) {
+        /* Cluster is outside of the image file or overlaps the header. */
+        return -1;
+    }
+
+    return cluster_off + offset;
 }
 
 static int cluster_remainder(BDRVParallelsState *s, int64_t sector_num,
@@ -703,18 +711,22 @@ parallels_check_outside_image(BlockDriverState *bs, BdrvCheckResult *res,
 {
     BDRVParallelsState *s = bs->opaque;
     uint32_t i;
-    int64_t off, high_off, size;
+    int64_t off, high_off, size, data_start_off;
 
     size = bdrv_co_getlength(bs->file->bs);
     if (size < 0) {
         res->check_errors++;
         return size;
     }
+    data_start_off = s->data_start << BDRV_SECTOR_BITS;
 
     high_off = 0;
     for (i = 0; i < s->bat_size; i++) {
         off = bat2sect(s, i) << BDRV_SECTOR_BITS;
-        if (off + s->cluster_size > size) {
+        if (off == 0) {
+            continue;
+        }
+        if (off < data_start_off || off + s->cluster_size > size) {
             fprintf(stderr, "%s cluster %u is outside image\n",
                     fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
             res->corruptions++;
@@ -1398,11 +1410,18 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
 
     for (i = 0; i < s->bat_size; i++) {
         sector = bat2sect(s, i);
+        if (sector == 0) {
+            continue; /* not allocated */
+        }
+        if (sector < data_start || sector + s->tracks > file_nb_sectors) {
+            /* Cluster is outside of the image file or overlaps the header. */
+            need_check = true;
+            continue;
+        }
         if (sector + s->tracks > s->data_end) {
             s->data_end = sector + s->tracks;
         }
     }
-    need_check = need_check || s->data_end > file_nb_sectors;
 
     if (!need_check) {
         ret = parallels_fill_used_bitmap(bs);
diff --git a/tests/qemu-iotests/tests/parallels-checks b/tests/qemu-iotests/tests/parallels-checks
index 9535024885..abd119bc7b 100755
--- a/tests/qemu-iotests/tests/parallels-checks
+++ b/tests/qemu-iotests/tests/parallels-checks
@@ -222,6 +222,64 @@ _img_info
 echo "== an unallocated cluster still reads as zeroes =="
 { $QEMU_IO -r -c "read -P 0x00 0 $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 =="
+poke_file_le "$TEST_IMG" $BAT_OFFSET 4 1000000
+
+echo "== read-only read must return zeroes, not an I/O error =="
+{ $QEMU_IO -r -c "read -P 0x00 0 $CLUSTER_SIZE" "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo "== write must allocate a fresh cluster instead of trusting the entry =="
+{ $QEMU_IO -c "write -P 0x77 0 $CLUSTER_SIZE" "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo "== file did not grow anywhere near the bogus offset =="
+file_size=`stat --printf="%s" "$TEST_IMG"`
+if [ "$file_size" -lt $((16 * 1024 * 1024)) ]; then
+    echo "file size sane: yes"
+else
+    echo "file size sane: no ($file_size bytes)"
+fi
+
+echo "== data reads back correctly =="
+{ $QEMU_IO -r -c "read -P 0x77 0 $CLUSTER_SIZE" "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+# Clear image, with a small cluster size so the BAT table itself spans
+# more than one cluster and there is room to point before data_off.
+_make_test_img -o cluster_size=512 65536
+
+SMALL_CLUSTER_SIZE=$(peek_file_le $TEST_IMG $CLUSTER_SIZE_OFFSET 4)
+SMALL_CLUSTER_SIZE=$((SMALL_CLUSTER_SIZE * 512))
+DATA_OFF=$(peek_file_le $TEST_IMG $DATA_OFF_OFFSET 4)
+echo "cluster size: $SMALL_CLUSTER_SIZE, data offset (sectors): $DATA_OFF"
+
+# Cluster index 1 starts at this byte offset, which must be < data_off
+# in sectors * 512 for this test to actually exercise the bug.
+VICTIM_OFFSET=$SMALL_CLUSTER_SIZE
+
+echo "== TEST BAT ENTRY POINTING BEFORE DATA AREA =="
+
+echo "== corrupt image: point first cluster into the BAT table itself =="
+poke_file_le "$TEST_IMG" $BAT_OFFSET 4 1
+
+echo "== qemu-img check detects it without repairing =="
+_check_test_img
+
+echo "== bytes at the victim offset before write =="
+echo "$(peek_file_le "$TEST_IMG" $VICTIM_OFFSET 4)"
+
+echo "== write must allocate a fresh cluster instead of clobbering the BAT =="
+{ $QEMU_IO -c "write -P 0x88 0 $SMALL_CLUSTER_SIZE" "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo "== bytes at the victim offset are unchanged =="
+echo "$(peek_file_le "$TEST_IMG" $VICTIM_OFFSET 4)"
+
+echo "== data reads back correctly =="
+{ $QEMU_IO -r -c "read -P 0x88 0 $SMALL_CLUSTER_SIZE" "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/tests/parallels-checks.out b/tests/qemu-iotests/tests/parallels-checks.out
index 6fb2014e8e..914616d023 100644
--- a/tests/qemu-iotests/tests/parallels-checks.out
+++ b/tests/qemu-iotests/tests/parallels-checks.out
@@ -140,4 +140,39 @@ virtual size: 4 MiB (4194304 bytes)
 == an unallocated cluster still reads as zeroes ==
 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 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 ==
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+== write must allocate a fresh cluster instead of trusting the entry ==
+Repairing cluster 0 is outside image
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+== file did not grow anywhere near the bogus offset ==
+file size sane: yes
+== data reads back correctly ==
+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=65536
+cluster size: 512, data offset (sectors): 2
+== TEST BAT ENTRY POINTING BEFORE DATA AREA ==
+== corrupt image: point first cluster into the BAT table itself ==
+== qemu-img check detects it without repairing ==
+ERROR cluster 0 is outside image
+
+1 errors were found on the image.
+Data may be corrupted, or further writes to the image may corrupt it.
+== bytes at the victim offset before write ==
+0
+== write must allocate a fresh cluster instead of clobbering the BAT ==
+Repairing cluster 0 is outside image
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+== bytes at the victim offset are unchanged ==
+0
+== data reads back correctly ==
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 *** done
-- 
2.53.0

[PULL v2 5/9] parallels: validate bitmap L1 table size before allocating it

[Denis V. Lunev]

parallels_load_bitmap() allocated the L1 table sized directly from
the untrusted l1_size field, only cross-checking it against the
bitmap's actual size after the allocation and the L1 table copy had
already happened.

Compute the expected size and reject a mismatch before touching the
allocator, instead of after.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Thomas Huth <thuth@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels-ext.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 3410daa620..97744c9696 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -70,20 +70,11 @@ parallels_load_bitmap_data(BlockDriverState *bs, const uint64_t *l1_table,
     uint64_t offset, limit;
     uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap);
     uint8_t *buf = NULL;
-    uint64_t i, tab_size =
-        DIV_ROUND_UP(bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size),
-                     s->cluster_size);
-
-    if (tab_size != l1_size) {
-        error_setg(errp, "Bitmap table size %" PRIu32 " does not correspond "
-                   "to bitmap size and cluster size. Expected %" PRIu64,
-                   l1_size, tab_size);
-        return -EINVAL;
-    }
+    uint64_t i;
 
     buf = qemu_blockalign(bs, s->cluster_size);
     limit = bdrv_dirty_bitmap_serialization_coverage(s->cluster_size, bitmap);
-    for (i = 0, offset = 0; i < tab_size; ++i, offset += limit) {
+    for (i = 0, offset = 0; i < l1_size; ++i, offset += limit) {
         uint64_t count = MIN(bm_size - offset, limit);
         uint64_t entry = l1_table[i];
 
@@ -124,12 +115,14 @@ static BdrvDirtyBitmap * GRAPH_RDLOCK
 parallels_load_bitmap(BlockDriverState *bs, uint8_t *data, size_t data_size,
                       Error **errp)
 {
+    BDRVParallelsState *s = bs->opaque;
     int ret;
     ParallelsDirtyBitmapFeature bf;
     g_autofree uint64_t *l1_table = NULL;
     BdrvDirtyBitmap *bitmap;
     QemuUUID uuid;
     char uuidstr[UUID_STR_LEN];
+    uint64_t bm_size, tab_size;
     int i;
 
     if (data_size < sizeof(bf)) {
@@ -164,6 +157,17 @@ parallels_load_bitmap(BlockDriverState *bs, uint8_t *data, size_t data_size,
         return NULL;
     }
 
+    bm_size = bdrv_dirty_bitmap_size(bitmap);
+    tab_size = DIV_ROUND_UP(
+        bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size),
+        s->cluster_size);
+    if (tab_size != bf.l1_size) {
+        error_setg(errp, "Bitmap table size %" PRIu32 " does not correspond "
+                   "to bitmap size and cluster size. Expected %" PRIu64,
+                   bf.l1_size, tab_size);
+        goto fail;
+    }
+
     l1_table = g_new(uint64_t, bf.l1_size);
     for (i = 0; i < bf.l1_size; i++, data += sizeof(uint64_t)) {
         l1_table[i] = ldq_le_p(data);
@@ -171,8 +175,7 @@ parallels_load_bitmap(BlockDriverState *bs, uint8_t *data, size_t data_size,
 
     ret = parallels_load_bitmap_data(bs, l1_table, bf.l1_size, bitmap, errp);
     if (ret < 0) {
-        bdrv_release_dirty_bitmap(bitmap);
-        return NULL;
+        goto fail;
     }
 
     /* We support format extension only for RO parallels images. */
@@ -180,6 +183,10 @@ parallels_load_bitmap(BlockDriverState *bs, uint8_t *data, size_t data_size,
     bdrv_dirty_bitmap_set_readonly(bitmap, true);
 
     return bitmap;
+
+fail:
+    bdrv_release_dirty_bitmap(bitmap);
+    return NULL;
 }
 
 static int GRAPH_RDLOCK
-- 
2.53.0

[PULL v2 6/9] parallels: skip loading a genuinely empty bitmap L1 table

[Denis V. Lunev]

parallels_load_bitmap_data() unconditionally calls
bdrv_dirty_bitmap_deserialize_finish() even when there is nothing to
deserialize, which hits an assertion in hbitmap
(hbitmap_iter_init: 'pos < hb->size') when the bitmap itself has
zero size, i.e. the disk is a zero-sector image.

Skip allocating, populating and loading the L1 table entirely when
l1_size == 0. This is safe only because the previous commit already
guarantees l1_size == 0 exclusively means the disk has 0 size.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Thomas Huth <thuth@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels-ext.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 97744c9696..704e16e1de 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -168,14 +168,17 @@ parallels_load_bitmap(BlockDriverState *bs, uint8_t *data, size_t data_size,
         goto fail;
     }
 
-    l1_table = g_new(uint64_t, bf.l1_size);
-    for (i = 0; i < bf.l1_size; i++, data += sizeof(uint64_t)) {
-        l1_table[i] = ldq_le_p(data);
-    }
+    if (bf.l1_size != 0) {
+        l1_table = g_new(uint64_t, bf.l1_size);
+        for (i = 0; i < bf.l1_size; i++, data += sizeof(uint64_t)) {
+            l1_table[i] = ldq_le_p(data);
+        }
 
-    ret = parallels_load_bitmap_data(bs, l1_table, bf.l1_size, bitmap, errp);
-    if (ret < 0) {
-        goto fail;
+        ret = parallels_load_bitmap_data(bs, l1_table, bf.l1_size, bitmap,
+                                         errp);
+        if (ret < 0) {
+            goto fail;
+        }
     }
 
     /* We support format extension only for RO parallels images. */
-- 
2.53.0

[PULL v2 7/9] parallels: avoid fatal abort on large bitmap L1 table

[Denis V. Lunev]

parallels_load_bitmap() allocated the L1 table with g_new(), which
aborts the whole process on allocation failure instead of returning
an error.

Use g_try_new() and fail the open normally.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Thomas Huth <thuth@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels-ext.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 704e16e1de..7f6ab6b0d2 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -169,7 +169,13 @@ parallels_load_bitmap(BlockDriverState *bs, uint8_t *data, size_t data_size,
     }
 
     if (bf.l1_size != 0) {
-        l1_table = g_new(uint64_t, bf.l1_size);
+        l1_table = g_try_new(uint64_t, bf.l1_size);
+        if (!l1_table) {
+            error_setg(errp, "Failed to allocate the bitmap L1 table "
+                       "(%" PRIu32 " entries)", bf.l1_size);
+            goto fail;
+        }
+
         for (i = 0; i < bf.l1_size; i++, data += sizeof(uint64_t)) {
             l1_table[i] = ldq_le_p(data);
         }
-- 
2.53.0

[PULL v2 8/9] parallels: validate BAT capacity against advertised disk size

[Denis V. Lunev]

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 <bea1e@proton.me>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3804
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Thomas Huth <thuth@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 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 abd119bc7b..d2a08049d9 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 =="
+_img_info
+
+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 914616d023..c33f3852a8 100644
--- a/tests/qemu-iotests/tests/parallels-checks.out
+++ b/tests/qemu-iotests/tests/parallels-checks.out
@@ -141,6 +141,13 @@ virtual size: 4 MiB (4194304 bytes)
 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.IMGFMT': 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

[PULL v2 9/9] MAINTAINERS: update parallels tree location

[Denis V. Lunev]

src.openvz.org is become unmaintained, point to the GitLab tree
instead.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index a28935c898..902db77218 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4393,7 +4393,7 @@ F: block/parallels.c
 F: block/parallels-ext.c
 F: docs/interop/parallels.rst
 F: docs/interop/prl-xml.rst
-T: git https://src.openvz.org/scm/~den/qemu.git parallels
+T: git https://gitlab.com/dlunev/qemu.git parallels
 
 qed
 M: Stefan Hajnoczi <stefanha@redhat.com>
-- 
2.53.0

Re: [PULL v2 0/9] parallels: fix reachable assertion and related bounds-checking gaps

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 116 bytes --]

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/11.1 for any user-visible changes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]