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

[Stefano Garzarella <sgarzare@redhat.com>]

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 <leonardi@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
>