None of the directive handlers that place data at a byte_offset within a parameter area validated that byte_offset actually falls within the parameter area's size. A malformed IGVM file with byte_offset > size would underflow the "size - byte_offset" computation used to determine remaining space, wrapping to a huge value and defeating the size check, then write out of bounds through param_entry->data + byte_offset.
Add qigvm_find_param_validate(), which looks up the parameter area, checks byte_offset against its size, and returns the offset-adjusted data pointer and remaining size via output parameters instead of the raw QIgvmParameterData entry. This keeps the byte_offset arithmetic in one place instead of repeating param_entry->data + byte_offset and param_entry->size - byte_offset at every call site. Use it in the vp-count and environment-info handlers, adding an explicit check that the fixed-size write fits in the remaining space. Signed-off-by: Luigi Leonardi <[email protected]> --- backends/igvm.c | 68 +++++++++++++++++++++++++++++++++++------- include/system/igvm-internal.h | 6 ++++ 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/backends/igvm.c b/backends/igvm.c index 7b7bdc72b7..8c07f2ce5e 100644 --- a/backends/igvm.c +++ b/backends/igvm.c @@ -101,6 +101,39 @@ qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index, return NULL; } +/* + * Look up a parameter area and check that param->byte_offset falls within + * it, in one step. On success, *data and *size are set to the + * offset-adjusted write location and remaining space within the parameter + * area, so callers never need to touch param->byte_offset themselves. + */ +bool +qigvm_find_param_validate(QIgvm *igvm, uint32_t parameter_area_index, + const IGVM_VHS_PARAMETER *param, + uint8_t **data, uint32_t *size, + Error **errp) +{ + QIgvmParameterData *param_entry; + + assert(data && size); + + param_entry = qigvm_find_param_entry(igvm, parameter_area_index, errp); + if (!param_entry) { + return false; + } + + 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 false; + } + + *data = param_entry->data + param->byte_offset; + *size = param_entry->size - param->byte_offset; + return true; +} + 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, @@ -682,17 +715,24 @@ 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; + uint8_t *data; + uint32_t size; uint32_t *vp_count; CPUState *cpu; - 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; + } + + if (sizeof(*vp_count) > 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 = (uint32_t *)data; *vp_count = 0; CPU_FOREACH(cpu) { @@ -707,17 +747,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; + uint8_t *data; + uint32_t size; IgvmEnvironmentInfo *environmental_state; - 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; + } + + if (sizeof(*environmental_state) > 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 = (IgvmEnvironmentInfo *)data; environmental_state->memory_is_shared = 1; return 0; diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h index 9e9fa1d9af..6526b5055c 100644 --- a/include/system/igvm-internal.h +++ b/include/system/igvm-internal.h @@ -81,4 +81,10 @@ QIgvmParameterData* qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index, Error **errp); +bool +qigvm_find_param_validate(QIgvm *igvm, uint32_t parameter_area_index, + const IGVM_VHS_PARAMETER *param, + uint8_t **data, uint32_t *size, + Error **errp); + #endif -- 2.55.0
