From: Luigi Leonardi <[email protected]>
Parameter directive handlers either ignore param->byte_offset,
overwriting each other when several parameters share one area, or use
it without validating it, letting a malformed IGVM file corrupt QEMU's
memory.
Add qigvm_get_param_data(), which validates byte_offset against the
parameter area size and returns the offset-adjusted pointer together
with the space left after it. Convert all parameter directive handlers
to use it and to check their writes against that remaining space, so
that no parameter can be written past the end of its area.
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")
(Mjt: 1c4bd8f13c is not in 11.0.x, corresponding change dropped)
Signed-off-by: Luigi Leonardi <[email protected]>
Reviewed-by: Stefano Garzarella <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Gerd Hoffmann <[email protected]>
(cherry picked from commit d9275e776e95ca04bd47bdf519cf1b1d9101b0ce)
Signed-off-by: Michael Tokarev <[email protected]>
diff --git a/backends/igvm.c b/backends/igvm.c
index 3a07f85db73..4cdbc5d43e9 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -77,6 +77,38 @@ qigvm_find_param_entry(QIgvm *igvm, uint32_t
parameter_area_index,
return NULL;
}
+/*
+ * Get parameter area data at byte_offset with bounds validation.
+ * On success, returns offset-adjusted data pointer and sets param_size
+ * to remaining space.
+ * Returns NULL on failure.
+ */
+uint8_t *
+qigvm_get_param_data(QIgvm *igvm, const IGVM_VHS_PARAMETER *param,
+ uint32_t *param_size,
+ Error **errp)
+{
+ QIgvmParameterData *param_entry;
+
+ assert(param_size);
+
+ param_entry = qigvm_find_param_entry(igvm, param->parameter_area_index,
+ errp);
+ if (!param_entry) {
+ return NULL;
+ }
+
+ if (param->byte_offset > param_entry->size) {
+ error_setg(errp,
+ "IGVM: byte_offset 0x%x exceeds parameter area size 0x%x",
+ param->byte_offset, param_entry->size);
+ return NULL;
+ }
+
+ *param_size = param_entry->size - param->byte_offset;
+ return param_entry->data + param->byte_offset;
+}
+
static int qigvm_directive_page_data(QIgvm *ctx, const uint8_t *header_data,
Error **errp);
static int qigvm_directive_vp_context(QIgvm *ctx, const uint8_t *header_data,
@@ -555,7 +587,7 @@ 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;
+ uint32_t param_size;
int max_entry_count;
int entry = 0;
IGVM_VHS_MEMORY_MAP_ENTRY *mm_entry;
@@ -576,14 +608,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) {
+ mm_entry =
+ (IGVM_VHS_MEMORY_MAP_ENTRY *)qigvm_get_param_data(ctx, param,
+ ¶m_size, errp);
+ if (!mm_entry) {
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);
retval = get_mem_map_entry(entry, &cgmm_entry, errp);
while (retval == 0) {
@@ -632,17 +664,22 @@ static int qigvm_directive_vp_count(QIgvm *ctx, const
uint8_t *header_data,
Error **errp)
{
const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
- QIgvmParameterData *param_entry;
+ uint32_t param_size;
uint32_t *vp_count;
CPUState *cpu;
- param_entry = qigvm_find_param_entry(ctx,
- param->parameter_area_index, errp);
- if (param_entry == NULL) {
+ vp_count = (uint32_t *)qigvm_get_param_data(ctx, param, ¶m_size, errp);
+ if (!vp_count) {
+ return -1;
+ }
+
+ if (sizeof(*vp_count) > param_size) {
+ error_setg(errp,
+ "IGVM: vp-count parameter exceeds parameter area "
+ "defined in IGVM file");
return -1;
}
- vp_count = (uint32_t *)(param_entry->data + param->byte_offset);
*vp_count = 0;
CPU_FOREACH(cpu)
{
@@ -657,17 +694,23 @@ static int qigvm_directive_environment_info(QIgvm *ctx,
Error **errp)
{
const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
- QIgvmParameterData *param_entry;
+ uint32_t param_size;
IgvmEnvironmentInfo *environmental_state;
- param_entry = qigvm_find_param_entry(ctx,
- param->parameter_area_index, errp);
- if (param_entry == NULL) {
+ environmental_state =
+ (IgvmEnvironmentInfo *)qigvm_get_param_data(ctx, param, ¶m_size,
+ errp);
+ if (!environmental_state) {
+ return -1;
+ }
+
+ if (sizeof(*environmental_state) > param_size) {
+ error_setg(errp,
+ "IGVM: environment-info parameter exceeds parameter area "
+ "defined in IGVM file");
return -1;
}
- environmental_state =
- (IgvmEnvironmentInfo *)(param_entry->data + param->byte_offset);
environmental_state->memory_is_shared = 1;
return 0;
diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h
index 7eb3792ed8c..1330ea3f6e0 100644
--- a/include/system/igvm-internal.h
+++ b/include/system/igvm-internal.h
@@ -75,4 +75,9 @@ QIgvmParameterData*
qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index,
Error **errp);
+uint8_t *
+qigvm_get_param_data(QIgvm *igvm, const IGVM_VHS_PARAMETER *param,
+ uint32_t *param_size,
+ Error **errp);
+
#endif
diff --git a/target/i386/igvm.c b/target/i386/igvm.c
index ad9bf87761f..9c2f8d51649 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 *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, ¶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.47.3
