Re: [PATCH] igvm: fix handling of optional variable header types

[Luigi Leonardi <leonardi@redhat.com>]

On Mon, May 25, 2026 at 07:37:55PM +0530, Ani Sinha wrote:
>
>
>> On 25 May 2026, at 4:26 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>>
>> The IGVM spec defines bit 31 of the variable header type as an
>> optional flag: if set, a loader that does not recognize the header
>> type may safely skip it. If clear, the loader must reject the file.
>>
>> Currently, all the types with the optional bit set are not
>> recognized as valid headers.
>>
>> Implement optional header handling by masking bit 31 before matching
>> against the handler table, and skip with a warning any unrecognized
>> header that has the optional bit set.
>>
>> Fixes: c1d466d267cf ("backends/igvm: Add IGVM loader and configuration")
>> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>> ---
>> backends/igvm.c | 24 +++++++++++++++++++++---
>> 1 file changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/backends/igvm.c b/backends/igvm.c
>> index c347d0c17e..408917f826 100644
>> --- a/backends/igvm.c
>> +++ b/backends/igvm.c
>> @@ -26,6 +26,7 @@
>> #include <igvm/igvm.h>
>> #include <igvm/igvm_defs.h>
>>
>> +#define IGVM_VHT_OPTIONAL_BIT (1U << 31)
>>
>> /*
>>  * Some directives are specific to particular confidential computing platforms.
>> @@ -139,8 +140,16 @@ static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp)
>>     const uint8_t *header_data;
>>     int result;
>>
>> +    /*
>> +     * Bit 31 of the variable header type indicates that the header is
>> +     * optional and can be safely ignored by a loader that does not
>> +     * support it. If the bit is clear, the file cannot be loaded.
>> +     * https://docs.rs/igvm_defs/0.4.0/igvm_defs/struct.IgvmVariableHeaderType.html
>> +     */
>> +    IgvmVariableHeaderType base_type = type & ~IGVM_VHT_OPTIONAL_BIT;
>> +
>>     for (handler = 0; handler < G_N_ELEMENTS(handlers); handler++) {
>> -        if (handlers[handler].type != type) {
>> +        if (handlers[handler].type != base_type) {
>>             continue;
>>         }
>>         header_handle = igvm_get_header(ctx->file, handlers[handler].section,
>> @@ -166,6 +175,13 @@ static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp)
>>         igvm_free_buffer(ctx->file, header_handle);
>>         return result;
>>     }
>> +
>> +    if (type & IGVM_VHT_OPTIONAL_BIT) {
>> +        warn_report("IGVM: Skipping unsupported optional header type 0x%"
>> +                    PRIX32, type);
>> +        return 0;
>> +    }
>> +
>
>I think this check should be at the top of the loop. If the bit is set, simply return with that warning message. No further processing is needed.

IIUC, in this way we would always skip optional parameter, right? My idea is to
always process optional parameters if we support it.

>
>
>>     error_setg(errp,
>>                "IGVM: Unknown header type encountered when processing file: "
>>                "(type 0x%X)",
>> @@ -787,7 +803,8 @@ static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp)
>>          header_index++) {
>>         IgvmVariableHeaderType typ = igvm_get_header_type(
>>             ctx->file, IGVM_HEADER_SECTION_PLATFORM, header_index);
>> -        if (typ == IGVM_VHT_SUPPORTED_PLATFORM) {
>> +        IgvmVariableHeaderType base_type = typ & ~IGVM_VHT_OPTIONAL_BIT;
>> +        if (base_type == IGVM_VHT_SUPPORTED_PLATFORM) {
>>             header_handle = igvm_get_header(
>>                 ctx->file, IGVM_HEADER_SECTION_PLATFORM, header_index);
>>             if (header_handle < 0) {
>> @@ -947,7 +964,8 @@ int qigvm_process_file(IgvmCfg *cfg, MachineState *machine_state,
>>          ctx.current_header_index++) {
>>         IgvmVariableHeaderType type = igvm_get_header_type(
>>             ctx.file, IGVM_HEADER_SECTION_DIRECTIVE, ctx.current_header_index);
>> -        if (!onlyVpContext || (type == IGVM_VHT_VP_CONTEXT)) {
>> +        IgvmVariableHeaderType base_type = type & ~IGVM_VHT_OPTIONAL_BIT;
>> +        if (!onlyVpContext || base_type == IGVM_VHT_VP_CONTEXT) {
>>             if (qigvm_handler(&ctx, type, errp) < 0) {
>>                 goto cleanup_parameters;
>>             }
>>
>> ---
>> base-commit: cbf877d67a812be17a9ce404a589e1bdf722c1f6
>> change-id: 20260525-igvm_optional-ca1592b613be
>>
>> Best regards,
>> --
>> Luigi Leonardi <leonardi@redhat.com>
>>
>