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

[Stefano Garzarella]

On Fri, Sep 04, 2026 at 01:42:56PM +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.

Switch these handlers to qigvm_find_param_validate(), introduced in
the previous patch, and use the offset-adjusted data pointer and
remaining size it returns instead of writing at param_entry->data and
sizing checks against param_entry->size directly. This both honors
byte_offset and validates it against the parameter area size before
it is used.

Do we need a Fixes tag?

The rest LGTM,
Stefano

Signed-off-by: Luigi Leonardi <leona...@redhat.com>
---
backends/igvm.c    | 24 ++++++++++++------------
target/i386/igvm.c | 12 ++++++------
2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/backends/igvm.c b/backends/igvm.c
index 8c07f2ce5e..4d622eac6a 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -638,7 +638,8 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const 
uint8_t *header_data,
    const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
    int (*get_mem_map_entry)(int index, ConfidentialGuestMemoryMapEntry *entry,
                             Error **errp) = NULL;
-    QIgvmParameterData *param_entry;
+    uint8_t *data;
+    uint32_t size;
    int max_entry_count;
    int entry = 0;
    IGVM_VHS_MEMORY_MAP_ENTRY *mm_entry;
@@ -659,14 +660,13 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const 
uint8_t *header_data,
    }

    /* Find the parameter area that should hold the memory map */
-    param_entry = qigvm_find_param_entry(ctx,
-                                         param->parameter_area_index, errp);
-    if (param_entry == NULL) {
+    if (!qigvm_find_param_validate(ctx, param->parameter_area_index, param,
+                                   &data, &size, errp)) {
        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 = size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
+    mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)data;

    retval = get_mem_map_entry(entry, &cgmm_entry, errp);
    while (retval == 0) {
@@ -860,12 +860,12 @@ static int qigvm_directive_device_tree(QIgvm *ctx, const 
uint8_t *header_data,
{
    const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
    g_autofree void *fdt_packed = NULL;
-    QIgvmParameterData *param_entry;
+    uint8_t *data;
+    uint32_t size;
    uint32_t fdt_size;

-    param_entry = qigvm_find_param_entry(ctx,
-                                         param->parameter_area_index, errp);
-    if (param_entry == NULL) {
+    if (!qigvm_find_param_validate(ctx, param->parameter_area_index, param,
+                                   &data, &size, errp)) {
        return -1;
    }

@@ -883,14 +883,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 > size) {
        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(data, fdt_packed, fdt_size);

    return 0;
}
diff --git a/target/i386/igvm.c b/target/i386/igvm.c
index ad9bf87761..db365cb80e 100644
--- a/target/i386/igvm.c
+++ b/target/i386/igvm.c
@@ -187,20 +187,20 @@ void qigvm_x86_bsp_reset(CPUX86State *env)
int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp)
{
    const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
-    QIgvmParameterData *param_entry;
+    uint8_t *data;
+    uint32_t size;
    int result = 0;

    /* Find the parameter area that should hold the MADT data */
-    param_entry = qigvm_find_param_entry(ctx,
-                                         param->parameter_area_index, errp);
-    if (param_entry == NULL) {
+    if (!qigvm_find_param_validate(ctx, param->parameter_area_index, param,
+                                   &data, &size, errp)) {
        return -1;
    }

    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 <= size) {
+        memcpy(data, madt->data, madt->len);
    } else {
        error_setg(
            errp,

--
2.55.0