[PATCH 0/2] igvm: fix byte_offset handling in parameter directives

[PATCH 0/2] igvm: fix byte_offset handling in parameter directives

[Luigi Leonardi]

Several IGVM directives (memory map, vp-count, environment info, MADT,
device tree) _can_ place their data at a byte_offset within a shared
parameter area, letting multiple parameters be packed into one area.

Patch 1 fixes three handlers that ignored byte_offset and always wrote
at the start of the buffer, corrupting earlier data when several
parameters share an area. Found while testing an IGVM file where a
device tree overflew into memory map.

Patch 2 adds validation of byte_offset in all five handlers, since a
malformed file with byte_offset > size would underflow the "remaining
space" computation and cause an out-of-bounds write.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
Luigi Leonardi (2):
      igvm: honor byte_offset when writing memory map, MADT and device tree
      igvm: validate byte_offset before using it in parameter directives

 backends/igvm.c                | 56 +++++++++++++++++++++++++++++++++++++++---
 include/system/igvm-internal.h |  4 +++
 target/i386/igvm.c             |  8 ++++--
 3 files changed, 62 insertions(+), 6 deletions(-)
---
base-commit: d2e570cc0f97b936902a5b1b86b73c0f5998b475
change-id: 20260902-fix_offset-a268cb12aafc

Best regards,
-- 
Luigi Leonardi <leonardi@redhat.com>

[PATCH 1/2] igvm: honor byte_offset when writing memory map, MADT and device tree

[Luigi Leonardi]

qigvm_directive_memory_map(), qigvm_directive_madt() and
qigvm_directive_device_tree() all wrote their data unconditionally at
the start of the parameter area's buffer, ignoring param->byte_offset
from the IGVM_VHS_PARAMETER header. This is harmless when a
directive's offset happens to be 0, but breaks for IGVM files that pack
multiple parameters into a single shared parameter area at different
offsets: a later directive would overwrite the data written by an earlier
one at the start of the buffer, corrupting it.

Use param_entry->data + param->byte_offset as the write location and
size the bounds checks against the remaining space in the buffer
rather than its full size.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
 backends/igvm.c    | 10 ++++++----
 target/i386/igvm.c |  4 ++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/backends/igvm.c b/backends/igvm.c
index 7b7bdc72b7..a6e3a58022 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -632,8 +632,10 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data,
         return -1;
     }
 
-    max_entry_count = param_entry->size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
-    mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_entry->data;
+    max_entry_count = (param_entry->size - param->byte_offset) /
+                      sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
+    mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)(param_entry->data +
+                                              param->byte_offset);
 
     retval = get_mem_map_entry(entry, &cgmm_entry, errp);
     while (retval == 0) {
@@ -837,14 +839,14 @@ static int qigvm_directive_device_tree(QIgvm *ctx, const uint8_t *header_data,
     }
 
     fdt_size = fdt_totalsize(fdt_packed);
-    if (fdt_size > param_entry->size) {
+    if (fdt_size > param_entry->size - param->byte_offset) {
         error_setg(errp,
                    "IGVM: device tree size exceeds parameter area"
                    " defined in IGVM file");
         return -1;
     }
 
-    memcpy(param_entry->data, fdt_packed, fdt_size);
+    memcpy(param_entry->data + param->byte_offset, fdt_packed, fdt_size);
 
     return 0;
 }
diff --git a/target/i386/igvm.c b/target/i386/igvm.c
index ad9bf87761..4d9d97385a 100644
--- a/target/i386/igvm.c
+++ b/target/i386/igvm.c
@@ -199,8 +199,8 @@ int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp)
 
     GArray *madt = acpi_build_madt_standalone(ctx->machine_state);
 
-    if (madt->len <= param_entry->size) {
-        memcpy(param_entry->data, madt->data, madt->len);
+    if (madt->len <= param_entry->size - param->byte_offset) {
+        memcpy(param_entry->data + param->byte_offset, madt->data, madt->len);
     } else {
         error_setg(
             errp,

-- 
2.55.0

[PATCH 2/2] igvm: validate byte_offset before using it in parameter directives

[Luigi Leonardi]

None of the directive handlers that place data at a byte_offset within
a parameter area validated that byte_offset actually falls within the
parameter area's size. A malformed IGVM file with byte_offset > size would
underflow the "size - byte_offset" computation used to determine
remaining space, wrapping to a huge value and defeating the size
check, then write out of bounds through param_entry->data +
byte_offset.

Add qigvm_param_offset_valid() to check byte_offset against the
parameter area size, and call it from each handler before doing any
arithmetic with byte_offset.

When possible, add an explicit check that fixed-size write fits
in the remaining space.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
 backends/igvm.c                | 46 ++++++++++++++++++++++++++++++++++++++++++
 include/system/igvm-internal.h |  4 ++++
 target/i386/igvm.c             |  4 ++++
 3 files changed, 54 insertions(+)

diff --git a/backends/igvm.c b/backends/igvm.c
index a6e3a58022..5d2364bf70 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -101,6 +101,22 @@ qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index,
     return NULL;
 }
 
+/*
+ * Check that byte_offset falls within the parameter area
+ */
+bool qigvm_param_offset_valid(const QIgvmParameterData *param_entry,
+                              const IGVM_VHS_PARAMETER *param,
+                              Error **errp)
+{
+    if (param->byte_offset > param_entry->size) {
+        error_setg(errp,
+                   "IGVM: byte_offset 0x%x exceeds parameter area size 0x%x",
+                   param->byte_offset, param_entry->size);
+        return false;
+    }
+    return true;
+}
+
 static int qigvm_directive_page_data(QIgvm *ctx, const uint8_t *header_data,
                                      Error **errp);
 static int qigvm_directive_vp_context(QIgvm *ctx, const uint8_t *header_data,
@@ -632,6 +648,10 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data,
         return -1;
     }
 
+    if (!qigvm_param_offset_valid(param_entry, param, errp)) {
+        return -1;
+    }
+
     max_entry_count = (param_entry->size - param->byte_offset) /
                       sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
     mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)(param_entry->data +
@@ -694,6 +714,17 @@ static int qigvm_directive_vp_count(QIgvm *ctx, const uint8_t *header_data,
         return -1;
     }
 
+    if (!qigvm_param_offset_valid(param_entry, param, errp)) {
+        return -1;
+    }
+
+    if (sizeof(*vp_count) > param_entry->size - param->byte_offset) {
+        error_setg(errp,
+                   "IGVM: vp-count parameter exceeds parameter area "
+                   "defined in IGVM file");
+        return -1;
+    }
+
     vp_count = (uint32_t *)(param_entry->data + param->byte_offset);
     *vp_count = 0;
     CPU_FOREACH(cpu)
@@ -718,6 +749,17 @@ static int qigvm_directive_environment_info(QIgvm *ctx,
         return -1;
     }
 
+    if (!qigvm_param_offset_valid(param_entry, param, errp)) {
+        return -1;
+    }
+
+    if (sizeof(*environmental_state) > param_entry->size - param->byte_offset) {
+        error_setg(errp,
+                   "IGVM: environment-info parameter exceeds parameter area "
+                   "defined in IGVM file");
+        return -1;
+    }
+
     environmental_state =
         (IgvmEnvironmentInfo *)(param_entry->data + param->byte_offset);
     environmental_state->memory_is_shared = 1;
@@ -825,6 +867,10 @@ static int qigvm_directive_device_tree(QIgvm *ctx, const uint8_t *header_data,
         return -1;
     }
 
+    if (!qigvm_param_offset_valid(param_entry, param, errp)) {
+        return -1;
+    }
+
     if (ctx->machine_state->fdt == NULL) {
         error_setg(errp, "IGVM: device tree not available");
         return -1;
diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h
index 9e9fa1d9af..7e0d1512ae 100644
--- a/include/system/igvm-internal.h
+++ b/include/system/igvm-internal.h
@@ -81,4 +81,8 @@ QIgvmParameterData*
 qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index,
                        Error **errp);
 
+bool qigvm_param_offset_valid(const QIgvmParameterData *param_entry,
+                              const IGVM_VHS_PARAMETER *param,
+                              Error **errp);
+
 #endif
diff --git a/target/i386/igvm.c b/target/i386/igvm.c
index 4d9d97385a..e758989d0d 100644
--- a/target/i386/igvm.c
+++ b/target/i386/igvm.c
@@ -197,6 +197,10 @@ int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp)
         return -1;
     }
 
+    if (!qigvm_param_offset_valid(param_entry, param, errp)) {
+        return -1;
+    }
+
     GArray *madt = acpi_build_madt_standalone(ctx->machine_state);
 
     if (madt->len <= param_entry->size - param->byte_offset) {

-- 
2.55.0

Re: [PATCH 1/2] igvm: honor byte_offset when writing memory map, MADT and device tree

[Stefano Garzarella]

On Wed, Sep 02, 2026 at 04:21:58PM +0200, Luigi Leonardi wrote:
>qigvm_directive_memory_map(), qigvm_directive_madt() and
>qigvm_directive_device_tree() all wrote their data unconditionally at
>the start of the parameter area's buffer, ignoring param->byte_offset
>from the IGVM_VHS_PARAMETER header. This is harmless when a
>directive's offset happens to be 0, but breaks for IGVM files that pack
>multiple parameters into a single shared parameter area at different
>offsets: a later directive would overwrite the data written by an earlier
>one at the start of the buffer, corrupting it.
>
>Use param_entry->data + param->byte_offset as the write location and
>size the bounds checks against the remaining space in the buffer
>rather than its full size.
>
>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>---
> backends/igvm.c    | 10 ++++++----
> target/i386/igvm.c |  4 ++--
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
>diff --git a/backends/igvm.c b/backends/igvm.c
>index 7b7bdc72b7..a6e3a58022 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -632,8 +632,10 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data,
>         return -1;
>     }
>
>-    max_entry_count = param_entry->size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
>-    mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_entry->data;
>+    max_entry_count = (param_entry->size - param->byte_offset) /

Should we validate that byte_offset >= size to avoid underflows?

>+                      sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
>+    mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)(param_entry->data +
>+                                              param->byte_offset);

Would it be better to add some helpers that return the size and the 
pointer (with validations as well)?

That way, we can avoid having to do this everywhere and also prevent 
future issues.

Stefano

>
>     retval = get_mem_map_entry(entry, &cgmm_entry, errp);
>     while (retval == 0) {
>@@ -837,14 +839,14 @@ static int qigvm_directive_device_tree(QIgvm *ctx, const uint8_t *header_data,
>     }
>
>     fdt_size = fdt_totalsize(fdt_packed);
>-    if (fdt_size > param_entry->size) {
>+    if (fdt_size > param_entry->size - param->byte_offset) {
>         error_setg(errp,
>                    "IGVM: device tree size exceeds parameter area"
>                    " defined in IGVM file");
>         return -1;
>     }
>
>-    memcpy(param_entry->data, fdt_packed, fdt_size);
>+    memcpy(param_entry->data + param->byte_offset, fdt_packed, fdt_size);
>
>     return 0;
> }
>diff --git a/target/i386/igvm.c b/target/i386/igvm.c
>index ad9bf87761..4d9d97385a 100644
>--- a/target/i386/igvm.c
>+++ b/target/i386/igvm.c
>@@ -199,8 +199,8 @@ int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp)
>
>     GArray *madt = acpi_build_madt_standalone(ctx->machine_state);
>
>-    if (madt->len <= param_entry->size) {
>-        memcpy(param_entry->data, madt->data, madt->len);
>+    if (madt->len <= param_entry->size - param->byte_offset) {
>+        memcpy(param_entry->data + param->byte_offset, madt->data, madt->len);
>     } else {
>         error_setg(
>             errp,
>
>-- 
>2.55.0
>

Re: [PATCH 2/2] igvm: validate byte_offset before using it in parameter directives

[Stefano Garzarella]

On Wed, Sep 02, 2026 at 04:21:59PM +0200, Luigi Leonardi wrote:
>None of the directive handlers that place data at a byte_offset within
>a parameter area validated that byte_offset actually falls within the
>parameter area's size. A malformed IGVM file with byte_offset > size would
>underflow the "size - byte_offset" computation used to determine
>remaining space, wrapping to a huge value and defeating the size
>check, then write out of bounds through param_entry->data +
>byte_offset.
>
>Add qigvm_param_offset_valid() to check byte_offset against the
>parameter area size, and call it from each handler before doing any
>arithmetic with byte_offset.
>
>When possible, add an explicit check that fixed-size write fits
>in the remaining space.
>
>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>---
> backends/igvm.c                | 46 ++++++++++++++++++++++++++++++++++++++++++
> include/system/igvm-internal.h |  4 ++++
> target/i386/igvm.c             |  4 ++++
> 3 files changed, 54 insertions(+)
>
>diff --git a/backends/igvm.c b/backends/igvm.c
>index a6e3a58022..5d2364bf70 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -101,6 +101,22 @@ qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index,
>     return NULL;
> }
>
>+/*
>+ * Check that byte_offset falls within the parameter area
>+ */
>+bool qigvm_param_offset_valid(const QIgvmParameterData *param_entry,
>+                              const IGVM_VHS_PARAMETER *param,
>+                              Error **errp)
>+{
>+    if (param->byte_offset > param_entry->size) {

Good, this is exaclt what I was suggesting in patch 1, but why 
introducing this check in a later commit? This will break bisectability 
IMO.

That said, I think we should move these checks in the helpers I 
suggested in patch 1, or maybe we should change qigvm_find_param_entry() 
or add wrapper around it to do everything (find, validate, return 
pointer/size).

Stefano

>+        error_setg(errp,
>+                   "IGVM: byte_offset 0x%x exceeds parameter area size 
>0x%x",
>+                   param->byte_offset, param_entry->size);
>+        return false;
>+    }
>+    return true;
>+}
>+
> static int qigvm_directive_page_data(QIgvm *ctx, const uint8_t *header_data,
>                                      Error **errp);
> static int qigvm_directive_vp_context(QIgvm *ctx, const uint8_t *header_data,
>@@ -632,6 +648,10 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data,
>         return -1;
>     }
>
>+    if (!qigvm_param_offset_valid(param_entry, param, errp)) {
>+        return -1;
>+    }
>+
>     max_entry_count = (param_entry->size - param->byte_offset) /
>                       sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
>     mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)(param_entry->data +
>@@ -694,6 +714,17 @@ static int qigvm_directive_vp_count(QIgvm *ctx, const uint8_t *header_data,
>         return -1;
>     }
>
>+    if (!qigvm_param_offset_valid(param_entry, param, errp)) {
>+        return -1;
>+    }
>+
>+    if (sizeof(*vp_count) > param_entry->size - param->byte_offset) {
>+        error_setg(errp,
>+                   "IGVM: vp-count parameter exceeds parameter area "
>+                   "defined in IGVM file");
>+        return -1;
>+    }
>+
>     vp_count = (uint32_t *)(param_entry->data + param->byte_offset);
>     *vp_count = 0;
>     CPU_FOREACH(cpu)
>@@ -718,6 +749,17 @@ static int qigvm_directive_environment_info(QIgvm *ctx,
>         return -1;
>     }
>
>+    if (!qigvm_param_offset_valid(param_entry, param, errp)) {
>+        return -1;
>+    }
>+
>+    if (sizeof(*environmental_state) > param_entry->size - param->byte_offset) {
>+        error_setg(errp,
>+                   "IGVM: environment-info parameter exceeds parameter area "
>+                   "defined in IGVM file");
>+        return -1;
>+    }
>+
>     environmental_state =
>         (IgvmEnvironmentInfo *)(param_entry->data + param->byte_offset);
>     environmental_state->memory_is_shared = 1;
>@@ -825,6 +867,10 @@ static int qigvm_directive_device_tree(QIgvm *ctx, const uint8_t *header_data,
>         return -1;
>     }
>
>+    if (!qigvm_param_offset_valid(param_entry, param, errp)) {
>+        return -1;
>+    }
>+
>     if (ctx->machine_state->fdt == NULL) {
>         error_setg(errp, "IGVM: device tree not available");
>         return -1;
>diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h
>index 9e9fa1d9af..7e0d1512ae 100644
>--- a/include/system/igvm-internal.h
>+++ b/include/system/igvm-internal.h
>@@ -81,4 +81,8 @@ QIgvmParameterData*
> qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index,
>                        Error **errp);
>
>+bool qigvm_param_offset_valid(const QIgvmParameterData *param_entry,
>+                              const IGVM_VHS_PARAMETER *param,
>+                              Error **errp);
>+
> #endif
>diff --git a/target/i386/igvm.c b/target/i386/igvm.c
>index 4d9d97385a..e758989d0d 100644
>--- a/target/i386/igvm.c
>+++ b/target/i386/igvm.c
>@@ -197,6 +197,10 @@ int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp)
>         return -1;
>     }
>
>+    if (!qigvm_param_offset_valid(param_entry, param, errp)) {
>+        return -1;
>+    }
>+
>     GArray *madt = acpi_build_madt_standalone(ctx->machine_state);
>
>     if (madt->len <= param_entry->size - param->byte_offset) {
>
>-- 
>2.55.0
>