[Qemu-devel] [PULL v2 26/37] block: vhdx - add region overlap detection for image files

[Stefan Hajnoczi <stefanha@redhat.com>]

From: Jeff Cody <jcody@redhat.com>

Regions in the image file cannot overlap - the log, region tables,
and metdata must all be unique and non-overlapping.

This adds region checking by means of a QLIST; there can be a variable
number of regions and metadata (there may be metadata or region tables
that we do not recognize / know about, but are not required).

This adds the capability to register a region for later checking, and
to check against registered regions for any overlap.

Also, if neither the BAT or Metadata region tables are found, return
error.

Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vhdx.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 block/vhdx.h |  9 +++++++
 2 files changed, 91 insertions(+)

diff --git a/block/vhdx.c b/block/vhdx.c
index 8fbfbd6..574ac4c 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -204,6 +204,50 @@ void vhdx_guid_generate(MSGUID *guid)
     memcpy(guid, uuid, sizeof(MSGUID));
 }
 
+/* Check for region overlaps inside the VHDX image */
+static int vhdx_region_check(BDRVVHDXState *s, uint64_t start, uint64_t length)
+{
+    int ret = 0;
+    uint64_t end;
+    VHDXRegionEntry *r;
+
+    end = start + length;
+    QLIST_FOREACH(r, &s->regions, entries) {
+        if (!((start >= r->end) || (end <= r->start))) {
+            ret = -EINVAL;
+            goto exit;
+        }
+    }
+
+exit:
+    return ret;
+}
+
+/* Register a region for future checks */
+static void vhdx_region_register(BDRVVHDXState *s,
+                                 uint64_t start, uint64_t length)
+{
+    VHDXRegionEntry *r;
+
+    r = g_malloc0(sizeof(*r));
+
+    r->start = start;
+    r->end = start + length;
+
+    QLIST_INSERT_HEAD(&s->regions, r, entries);
+}
+
+/* Free all registered regions */
+static void vhdx_region_unregister_all(BDRVVHDXState *s)
+{
+    VHDXRegionEntry *r, *r_next;
+
+    QLIST_FOREACH_SAFE(r, &s->regions, entries, r_next) {
+        QLIST_REMOVE(r, entries);
+        g_free(r);
+    }
+}
+
 /*
  * Per the MS VHDX Specification, for every VHDX file:
  *      - The header section is fixed size - 1 MB
@@ -389,6 +433,9 @@ static int vhdx_parse_header(BlockDriverState *bs, BDRVVHDXState *s)
         }
     }
 
+    vhdx_region_register(s, s->headers[s->curr_header]->log_offset,
+                            s->headers[s->curr_header]->log_length);
+
     ret = 0;
 
     goto exit;
@@ -452,6 +499,15 @@ static int vhdx_open_region_tables(BlockDriverState *bs, BDRVVHDXState *s)
         le32_to_cpus(&rt_entry.length);
         le32_to_cpus(&rt_entry.data_bits);
 
+        /* check for region overlap between these entries, and any
+         * other memory regions in the file */
+        ret = vhdx_region_check(s, rt_entry.file_offset, rt_entry.length);
+        if (ret < 0) {
+            goto fail;
+        }
+
+        vhdx_region_register(s, rt_entry.file_offset, rt_entry.length);
+
         /* see if we recognize the entry */
         if (guid_eq(rt_entry.guid, bat_guid)) {
             /* must be unique; if we have already found it this is invalid */
@@ -482,6 +538,12 @@ static int vhdx_open_region_tables(BlockDriverState *bs, BDRVVHDXState *s)
             goto fail;
         }
     }
+
+    if (!bat_rt_found || !metadata_rt_found) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
     ret = 0;
 
 fail:
@@ -751,6 +813,7 @@ static void vhdx_close(BlockDriverState *bs)
     error_free(s->migration_blocker);
     qemu_vfree(s->log.hdr);
     s->log.hdr = NULL;
+    vhdx_region_unregister_all(s);
 }
 
 static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
@@ -768,6 +831,7 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
     s->first_visible_write = true;
 
     qemu_co_mutex_init(&s->lock);
+    QLIST_INIT(&s->regions);
 
     /* validate the file signature */
     ret = bdrv_pread(bs->file, 0, &signature, sizeof(uint64_t));
@@ -842,8 +906,26 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
+    uint64_t payblocks = s->chunk_ratio;
+    /* endian convert, and verify populated BAT field file offsets against
+     * region table and log entries */
     for (i = 0; i < s->bat_entries; i++) {
         le64_to_cpus(&s->bat[i]);
+        if (payblocks--) {
+            /* payload bat entries */
+            if ((s->bat[i] & VHDX_BAT_STATE_BIT_MASK) ==
+                    PAYLOAD_BLOCK_FULL_PRESENT) {
+                ret = vhdx_region_check(s, s->bat[i] & VHDX_BAT_FILE_OFF_MASK,
+                                        s->block_size);
+                if (ret < 0) {
+                    goto fail;
+                }
+            }
+        } else {
+            payblocks = s->chunk_ratio;
+            /* Once differencing files are supported, verify sector bitmap
+             * blocks here */
+        }
     }
 
     if (flags & BDRV_O_RDWR) {
diff --git a/block/vhdx.h b/block/vhdx.h
index 584ebec..d906559 100644
--- a/block/vhdx.h
+++ b/block/vhdx.h
@@ -230,6 +230,7 @@ typedef struct QEMU_PACKED VHDXLogDataSector {
    other bits are reserved */
 #define VHDX_BAT_STATE_BIT_MASK 0x07
 #define VHDX_BAT_FILE_OFF_BITS (64 - 44)
+#define VHDX_BAT_FILE_OFF_MASK  0xFFFFFFFFFFF00000 /* upper 44 bits */
 typedef uint64_t VHDXBatEntry;
 
 /* ---- METADATA REGION STRUCTURES ---- */
@@ -334,6 +335,12 @@ typedef struct VHDXLogEntries {
     uint32_t tail;
 } VHDXLogEntries;
 
+typedef struct VHDXRegionEntry {
+    uint64_t start;
+    uint64_t end;
+    QLIST_ENTRY(VHDXRegionEntry) entries;
+} VHDXRegionEntry;
+
 typedef struct BDRVVHDXState {
     CoMutex lock;
 
@@ -374,6 +381,8 @@ typedef struct BDRVVHDXState {
     VHDXParentLocatorEntry *parent_entries;
 
     Error *migration_blocker;
+
+    QLIST_HEAD(VHDXRegionHead, VHDXRegionEntry) regions;
 } BDRVVHDXState;
 
 void vhdx_guid_generate(MSGUID *guid);
-- 
1.8.3.1