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. Signed-off-by: Luigi Leonardi <[email protected]> --- 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
