[PATCH] igvm: Report error on missing parameter area in directive handlers

Luigi Leonardi posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260610-igvm._5Ferror-v1-1-59d133a69f2f@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Ani Sinha <anisinha@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Zhao Liu <zhao1.liu@intel.com>
There is a newer version of this series
backends/igvm.c    | 17 ++++++++++++-----
target/i386/igvm.c |  4 +++-
2 files changed, 15 insertions(+), 6 deletions(-)
[PATCH] igvm: Report error on missing parameter area in directive handlers
Posted by Luigi Leonardi 1 month, 2 weeks ago
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 it
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. See it as a form
of defensive programming.

Report the error with error_setg() and return -1 instead.
Also remove the warn_report() from `qigvm_find_param_entry()`
to avoid double error reporting.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
 backends/igvm.c    | 17 ++++++++++++-----
 target/i386/igvm.c |  4 +++-
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/backends/igvm.c b/backends/igvm.c
index c347d0c17e..2569c4a9f2 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -71,7 +71,6 @@ 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);
     return NULL;
 }
 
@@ -502,7 +501,9 @@ static int qigvm_directive_parameter_insert(QIgvm *ctx,
 
     param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index);
     if (param_entry == NULL) {
-        return 0;
+        error_setg(errp, "IGVM: parameter area index %u not found",
+                   param->parameter_area_index);
+        return -1;
     }
 
     region = qigvm_prepare_memory(ctx, param->gpa, param_entry->size,
@@ -575,7 +576,9 @@ 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);
     if (param_entry == NULL) {
-        return 0;
+        error_setg(errp, "IGVM: parameter area index %u not found",
+                   param->parameter_area_index);
+        return -1;
     }
 
     max_entry_count = param_entry->size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
@@ -634,7 +637,9 @@ static int qigvm_directive_vp_count(QIgvm *ctx, const uint8_t *header_data,
 
     param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index);
     if (param_entry == NULL) {
-        return 0;
+        error_setg(errp, "IGVM: parameter area index %u not found",
+                   param->parameter_area_index);
+        return -1;
     }
 
     vp_count = (uint32_t *)(param_entry->data + param->byte_offset);
@@ -657,7 +662,9 @@ static int qigvm_directive_environment_info(QIgvm *ctx,
 
     param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index);
     if (param_entry == NULL) {
-        return 0;
+        error_setg(errp, "IGVM: parameter area index %u not found",
+                   param->parameter_area_index);
+        return -1;
     }
 
     environmental_state =
diff --git a/target/i386/igvm.c b/target/i386/igvm.c
index f41b498b89..e59511a2aa 100644
--- a/target/i386/igvm.c
+++ b/target/i386/igvm.c
@@ -193,7 +193,9 @@ int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp)
     /* Find the parameter area that should hold the MADT data */
     param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index);
     if (param_entry == NULL) {
-        return 0;
+        error_setg(errp, "IGVM: parameter area index %u not found",
+                   param->parameter_area_index);
+        return -1;
     }
 
     GArray *madt = acpi_build_madt_standalone(ctx->machine_state);

---
base-commit: 29c042c6e9d4a09d4a0ac3fa54aeb7ee08ce0bdc
change-id: 20260610-igvm_error-ce8e8b2ef70e

Best regards,
-- 
Luigi Leonardi <leonardi@redhat.com>


Re: [PATCH] igvm: Report error on missing parameter area in directive handlers
Posted by Gerd Hoffmann 1 month, 1 week ago
On Wed, Jun 10, 2026 at 11:44:21AM +0200, Luigi Leonardi wrote:
> 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 it
> 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. See it as a form
> of defensive programming.
> 
> Report the error with error_setg() and return -1 instead.
> Also remove the warn_report() from `qigvm_find_param_entry()`
> to avoid double error reporting.
> 
> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
> ---
>  backends/igvm.c    | 17 ++++++++++++-----
>  target/i386/igvm.c |  4 +++-
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/backends/igvm.c b/backends/igvm.c
> index c347d0c17e..2569c4a9f2 100644
> --- a/backends/igvm.c
> +++ b/backends/igvm.c
> @@ -71,7 +71,6 @@ 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);
>      return NULL;
>  }

When passing down errp to qigvm_find_param_entry() we can call
error_setg here instead duplicating it for each qigvm_find_param_entry
call.

take care,
  Gerd