Parameter areas are how an IGVM file tells QEMU to allocate buffers
for runtime information the guest needs — VP count, memory map,
MADT and so on. Usage directives reference a parameter area by index
to tell QEMU where to write each piece of data. If the index doesn't
match any declared parameter area, the data has nowhere to go and
should be treated as an error.

The directive handlers that look up a parameter area all return 0
(success) when `qigvm_find_param_entry()` can't find it. Therefore,
the load succeeds but the guest never gets the expected parameters.

Note that the IGVM library already validates parameter area indices
when the file is loaded, so this path should only be reachable with
a malformed file that bypassed library validation. This is defensive
programming against that case.

Report the error with error_setg() and return -1 instead.

Signed-off-by: Luigi Leonardi <[email protected]>
---
Changes in v2:
- Moved error_setg to qigvm_find_param_entry [Gerd]
- Rebased to latest upstream
- Link to v1: 
https://lore.kernel.org/qemu-devel/[email protected]
---
 backends/igvm.c                | 26 ++++++++++++++++----------
 include/system/igvm-internal.h |  3 ++-
 target/i386/igvm.c             |  5 +++--
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/backends/igvm.c b/backends/igvm.c
index 3f4b97a5d4..a5a2a4eccc 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -81,7 +81,8 @@ struct QEMU_PACKED sev_id_authentication {
 #define IGVM_SEV_ID_BLOCK_VERSION 1
 
 QIgvmParameterData*
-qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index)
+qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index,
+                       Error **errp)
 {
     QIgvmParameterData *param_entry;
     QTAILQ_FOREACH(param_entry, &igvm->parameter_data, next)
@@ -90,7 +91,8 @@ qigvm_find_param_entry(QIgvm *igvm, uint32_t 
parameter_area_index)
             return param_entry;
         }
     }
-    warn_report("IGVM: No parameter area for index %u", parameter_area_index);
+    error_setg(errp, "IGVM: parameter area index %u not found",
+                   parameter_area_index);
     return NULL;
 }
 
@@ -528,9 +530,10 @@ static int qigvm_directive_parameter_insert(QIgvm *ctx,
         return 0;
     }
 
-    param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index);
+    param_entry = qigvm_find_param_entry(ctx,
+                                         param->parameter_area_index, errp);
     if (param_entry == NULL) {
-        return 0;
+        return -1;
     }
 
     region = qigvm_prepare_memory(ctx, param->gpa, param_entry->size,
@@ -601,9 +604,10 @@ 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);
+    param_entry = qigvm_find_param_entry(ctx,
+                                         param->parameter_area_index, errp);
     if (param_entry == NULL) {
-        return 0;
+        return -1;
     }
 
     max_entry_count = param_entry->size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
@@ -660,9 +664,10 @@ static int qigvm_directive_vp_count(QIgvm *ctx, const 
uint8_t *header_data,
     uint32_t *vp_count;
     CPUState *cpu;
 
-    param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index);
+    param_entry = qigvm_find_param_entry(ctx,
+                                         param->parameter_area_index, errp);
     if (param_entry == NULL) {
-        return 0;
+        return -1;
     }
 
     vp_count = (uint32_t *)(param_entry->data + param->byte_offset);
@@ -683,9 +688,10 @@ static int qigvm_directive_environment_info(QIgvm *ctx,
     QIgvmParameterData *param_entry;
     IgvmEnvironmentInfo *environmental_state;
 
-    param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index);
+    param_entry = qigvm_find_param_entry(ctx,
+                                         param->parameter_area_index, errp);
     if (param_entry == NULL) {
-        return 0;
+        return -1;
     }
 
     environmental_state =
diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h
index 7f131c4d03..7eb3792ed8 100644
--- a/include/system/igvm-internal.h
+++ b/include/system/igvm-internal.h
@@ -72,6 +72,7 @@ struct QIgvm {
 IgvmHandle qigvm_file_init(char *filename, Error **errp);
 
 QIgvmParameterData*
-qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index);
+qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index,
+                       Error **errp);
 
 #endif
diff --git a/target/i386/igvm.c b/target/i386/igvm.c
index f41b498b89..ad9bf87761 100644
--- a/target/i386/igvm.c
+++ b/target/i386/igvm.c
@@ -191,9 +191,10 @@ int qigvm_directive_madt(QIgvm *ctx, const uint8_t 
*header_data, Error **errp)
     int result = 0;
 
     /* Find the parameter area that should hold the MADT data */
-    param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index);
+    param_entry = qigvm_find_param_entry(ctx,
+                                         param->parameter_area_index, errp);
     if (param_entry == NULL) {
-        return 0;
+        return -1;
     }
 
     GArray *madt = acpi_build_madt_standalone(ctx->machine_state);

---
base-commit: c7cf7c810153d6f5f31aa2d5c0dee9087f6b4dff
change-id: 20260610-igvm_error-ce8e8b2ef70e

Best regards,
-- 
Luigi Leonardi <[email protected]>


Reply via email to