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_get_param_data(), introduced in
the previous patch, and use the offset-adjusted data pointer it
returns and the remaining size it sets 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.
Fixes: c1d466d267 ("backends/igvm: Add IGVM loader and configuration")
Fixes: dea1f68a5c ("igvm: Fill MADT IGVM parameter field on x86_64")
Fixes: 1c4bd8f13c ("igvm: add device tree parameter support")
Signed-off-by: Luigi Leonardi <[email protected]>
---
backends/igvm.c | 26 ++++++++++++++------------
target/i386/igvm.c | 13 +++++++------
2 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/backends/igvm.c b/backends/igvm.c
index a8340d7d55..b3ff6cba4a 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 *param_data;
+ uint32_t param_size;
int max_entry_count;
int entry = 0;
IGVM_VHS_MEMORY_MAP_ENTRY *mm_entry;
@@ -659,14 +660,14 @@ 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) {
+ param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
+ param, ¶m_size, errp);
+ if (!param_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_size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
+ mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_data;
retval = get_mem_map_entry(entry, &cgmm_entry, errp);
while (retval == 0) {
@@ -862,12 +863,13 @@ 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 *param_data;
+ uint32_t param_size;
uint32_t fdt_size;
- param_entry = qigvm_find_param_entry(ctx,
- param->parameter_area_index, errp);
- if (param_entry == NULL) {
+ param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
+ param, ¶m_size, errp);
+ if (!param_data) {
return -1;
}
@@ -885,14 +887,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_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(param_data, fdt_packed, fdt_size);
return 0;
}
diff --git a/target/i386/igvm.c b/target/i386/igvm.c
index ad9bf87761..6ae662b033 100644
--- a/target/i386/igvm.c
+++ b/target/i386/igvm.c
@@ -187,20 +187,21 @@ 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 *param_data;
+ uint32_t param_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) {
+ param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
+ param, ¶m_size, errp);
+ if (!param_data) {
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 <= param_size) {
+ memcpy(param_data, madt->data, madt->len);
} else {
error_setg(
errp,
--
2.55.0