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

[Stefano Garzarella <sgarzare@redhat.com>]

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
>