Re: [PATCH 2/3] efi: Protect against unnecessary image unloading

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.09.2025 10:24, Gerald Elder-Vass wrote:
> @@ -1078,11 +1078,12 @@ static void __init efi_verify_kernel(EFI_HANDLE 
> ImageHandle)
>              verified = true;
>  
>          /*
> -         * Always unload the image.  We only needed LoadImage() to perform
> -         * verification anyway, and in the case of a failure there may still
> -         * be cleanup needing to be performed.
> +         * If the kernel was loaded, unload it. We only needed LoadImage() to
> +         * perform verification anyway, and in the case of a failure there 
> may
> +         * still be cleanup needing to be performed.
>           */
> -        shim_loader->UnloadImage(loaded_kernel);
> +        if ( loaded_kernel )
> +            shim_loader->UnloadImage(loaded_kernel);
>      }

To me this looks as odd as the earlier unconditional unloading. How would a
halfway sane implementation of LoadImage() return an error, but require
subsequent cleanup (and set what the last function argument points at to
non-NULL)? Unless explicitly specified otherwise, my expectation would be
that upon failure loaded_kernel could have any arbitrary value, possibly
entirely unsuitable to pass to UnloadImage().

Jan