[PATCH v6 16/25] parallels: reject a bitmap L1 entry outside the data area

From: Denis V. Lunev <[email protected]>

Nothing checks where a bitmap L1 entry points. The entry is turned into
an offset and the cluster is read from there, so the image decides which
part of the file, if any, is deserialized as bitmap data.

A short read on the protocol node is zero filled rather than refused, so
an entry beyond the end of the file does not even fail: the bitmap
quietly loads as completely clean. An entry below data_off deserializes
the header and the BAT as bitmap data instead. Both cases used to be
harmless in the sense that the extension was only parsed for read-only
images, but the bitmap becomes writable and is stored back as
authoritative once the image can be opened read-write.

Bound the entry the way parallels_check_outside_image() bounds a BAT
entry: it has to address the data area of the image file. The offset
computation is bounded first, as the entry is a 64 bit value coming from
the image and the shift by BDRV_SECTOR_BITS would overflow.

The overflow case was reported as an I/O error before, so the test
expectation changes along with it.

Fixes: baefd977002e ("parallels: support bitmap extension for read-only mode")
Cc: Stefan Hajnoczi <[email protected]>
Signed-off-by: Denis V. Lunev <[email protected]>
---
 block/parallels-ext.c                         | 30 ++++++++++++++++++-
 .../qemu-iotests/tests/parallels-read-bitmap  | 23 +++++++++++---
 .../tests/parallels-read-bitmap.out           |  6 +++-
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 6a889d86fa..17445d183b 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -71,9 +71,17 @@ parallels_load_bitmap_data(BlockDriverState *bs, const uint64_t *l1_table,
     int ret = 0;
     uint64_t offset, limit;
     uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap);
+    int64_t file_size, data_start_off;
     uint8_t *buf = NULL;
     uint64_t i;
 
+    file_size = bdrv_getlength(bs->file->bs);
+    if (file_size < 0) {
+        error_setg_errno(errp, -file_size, "Failed to get image file length");
+        return file_size;
+    }
+    data_start_off = s->data_start << BDRV_SECTOR_BITS;
+
     buf = qemu_try_blockalign(bs->file->bs, s->cluster_size);
     if (!buf) {
         error_setg(errp, "Failed to allocate a bitmap data cluster");
@@ -101,7 +109,27 @@ parallels_load_bitmap_data(BlockDriverState *bs, const uint64_t *l1_table,
         if (entry == 1) {
             bdrv_dirty_bitmap_deserialize_ones(bitmap, offset, count, false);
         } else {
-            int64_t host_off = entry << BDRV_SECTOR_BITS;
+            int64_t host_off;
+
+            if (entry > INT64_MAX / BDRV_SECTOR_SIZE) {
+                error_setg(errp, "Bitmap L1 entry %" PRIu64 " is out of range",
+                           i);
+                ret = -EINVAL;
+                goto finish;
+            }
+            host_off = entry * BDRV_SECTOR_SIZE;
+            if (host_off < data_start_off) {
+                error_setg(errp, "Bitmap L1 entry %" PRIu64 " points before "
+                           "the data area of the image", i);
+                ret = -EINVAL;
+                goto finish;
+            }
+            if (host_off > file_size - (int64_t)s->cluster_size) {
+                error_setg(errp, "Bitmap L1 entry %" PRIu64 " points outside "
+                           "the image file", i);
+                ret = -EINVAL;
+                goto finish;
+            }
 
             ret = bdrv_pread(bs->file, host_off, s->cluster_size, buf, 0);
             if (ret < 0) {
diff --git a/tests/qemu-iotests/tests/parallels-read-bitmap b/tests/qemu-iotests/tests/parallels-read-bitmap
index c9652fcf3a..ee071714a5 100755
--- a/tests/qemu-iotests/tests/parallels-read-bitmap
+++ b/tests/qemu-iotests/tests/parallels-read-bitmap
@@ -93,9 +93,10 @@ def extension(body, magic=EXT_MAGIC, checksum=True):
     return struct.pack('<Q16s', magic, csum) + body
 
 
-def parallels_header(tracks, bat_entries, nb_sectors, ext_sector):
+def parallels_header(tracks, bat_entries, nb_sectors, ext_sector, data_off=1):
     return struct.pack('<16sIIIIIQIIIQ', b'WithouFreSpacExt', 2, 16, 0,
-                       tracks, bat_entries, nb_sectors, 0, 1, 0, ext_sector)
+                       tracks, bat_entries, nb_sectors, 0, data_off, 0,
+                       ext_sector)
 
 
 def write_sparse_image(body, tracks, nb_sectors, bat_entries, ext_sector):
@@ -133,12 +134,14 @@ def report(name):
             log(line.strip())
 
 
-def check(name, ext, tracks=1):
-    header = parallels_header(tracks, SECTORS, SECTORS, EXT_SECTOR)
+def check(name, ext, tracks=1, data_off=1, pad=0):
+    header = parallels_header(tracks, SECTORS, SECTORS, EXT_SECTOR, data_off)
     with open(crafted, 'wb') as f:
         f.write(header)
         f.write(bytes(EXT_SECTOR * 512 - len(header)))   # BAT, unallocated
         f.write(ext)
+        if pad:
+            f.truncate(EXT_SECTOR * 512 + tracks * 512 * (1 + pad))
 
     report(name)
 
@@ -192,6 +195,18 @@ bf = bitmap_feature([0xffffffffffffffff])
 check('bitmap L1 entry overflows',
       extension(feature(BITMAP_MAGIC, len(bf)) + bf + feature(0, 0)))
 
+# A cluster which is not there reads back as zeroes, as short reads on the
+# protocol node are zero filled, so the bitmap would silently load as clean.
+bf = bitmap_feature([8])
+check('bitmap L1 entry past the image file',
+      extension(feature(BITMAP_MAGIC, len(bf)) + bf + feature(0, 0)))
+
+# An entry pointing at the header would deserialize the header as bitmap data.
+bf = bitmap_feature([4])
+check('bitmap L1 entry inside the header',
+      extension(feature(BITMAP_MAGIC, len(bf)) + bf + feature(0, 0)),
+      data_off=8, pad=16)
+
 # Largest cluster_size parallels_open() accepts, about 2 GiB.
 check('cluster_size beyond the file size',
       extension(feature(0, 0)), tracks=0x7fffffff // 513)
diff --git a/tests/qemu-iotests/tests/parallels-read-bitmap.out b/tests/qemu-iotests/tests/parallels-read-bitmap.out
index 13b10af5a4..ebfc0acb4d 100644
--- a/tests/qemu-iotests/tests/parallels-read-bitmap.out
+++ b/tests/qemu-iotests/tests/parallels-read-bitmap.out
@@ -33,7 +33,11 @@ qemu-img: Could not open 'TEST_DIR/PID-crafted': Invalid bitmap granularity 1536
 --- bitmap granularity 8388608
 qemu-img: Could not open 'TEST_DIR/PID-crafted': Invalid bitmap granularity 4294967296, expected a power of two of at least 512 bytes
 --- bitmap L1 entry overflows
-qemu-img: Could not open 'TEST_DIR/PID-crafted': Failed to read bitmap data cluster: Input/output error
+qemu-img: Could not open 'TEST_DIR/PID-crafted': Bitmap L1 entry 0 is out of range
+--- bitmap L1 entry past the image file
+qemu-img: Could not open 'TEST_DIR/PID-crafted': Bitmap L1 entry 0 points outside the image file
+--- bitmap L1 entry inside the header
+qemu-img: Could not open 'TEST_DIR/PID-crafted': Bitmap L1 entry 0 points before the data area of the image
 --- cluster_size beyond the file size
 image opened
 qemu-img: warning: Dropping the Format Extension of node 'NODE_NAME', which does not look like one: Format Extension is outside the image file
-- 
2.53.0