Re: [XEN PATCH] efi: Use Shim's LoadImage to verify the Dom0 kernel

["Yann Sionneau" <yann.sionneau@xxxxxxxxxx>]

On 9/1/25 17:37, Gerald Elder-Vass wrote:
> The existing Verify functionality of the Shim lock protocol is
> deprecated and will be removed, instead we must use the LoadImage
> interface to perform the verification.
> 
> When the loading is successful we won't be using the newly loaded image
> (as of yet) so we must then immediately unload the image to clean up.
> 
> Signed-off-by: Gerald Elder-Vass <gerald.elder-vass@xxxxxxxxx>
> Signed-off-by: Kevin Lampis <kevin.lampis@xxxxxxxxx>
> ---
>   xen/common/efi/boot.c | 39 +++++++++++++++++++++++++++------------
>   1 file changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index 453b1ba099cd..67e7220d8fa3 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -36,8 +36,8 @@
>   
>   #define SMBIOS3_TABLE_GUID \
>     { 0xf2fd1544U, 0x9794, 0x4a2c, {0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 
> 0x94} }
> -#define SHIM_LOCK_PROTOCOL_GUID \
> -  { 0x605dab50U, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 
> 0x23} }
> +#define SHIM_IMAGE_LOADER_GUID \
> +  { 0x1f492041U, 0xfadb, 0x4e59, {0x9e, 0x57, 0x7c, 0xaf, 0xe7, 0x3a, 0x55, 
> 0xab} }
>   #define APPLE_PROPERTIES_PROTOCOL_GUID \
>     { 0x91bd12feU, 0xf6c3, 0x44fb, {0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 
> 0xe0} }
>   #define EFI_SYSTEM_RESOURCE_TABLE_GUID    \
> @@ -66,9 +66,12 @@ typedef EFI_STATUS
>       IN const VOID *Buffer,
>       IN UINT32 Size);
>   
> -typedef struct {
> -    EFI_SHIM_LOCK_VERIFY Verify;
> -} EFI_SHIM_LOCK_PROTOCOL;
> +typedef struct _SHIM_IMAGE_LOADER {
> +    EFI_IMAGE_LOAD LoadImage;
> +    EFI_IMAGE_START StartImage;
> +    EFI_EXIT Exit;
> +    EFI_IMAGE_UNLOAD UnloadImage;
> +} SHIM_IMAGE_LOADER;
>   
>   struct _EFI_APPLE_PROPERTIES;
>   
> @@ -1333,13 +1336,13 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE 
> ImageHandle,
>                                         EFI_SYSTEM_TABLE *SystemTable)
>   {
>       static EFI_GUID __initdata loaded_image_guid = LOADED_IMAGE_PROTOCOL;
> -    static EFI_GUID __initdata shim_lock_guid = SHIM_LOCK_PROTOCOL_GUID;
>       EFI_LOADED_IMAGE *loaded_image;
>       EFI_STATUS status;
> +    EFI_HANDLE loaded_kernel;
>       unsigned int i;
>       CHAR16 *file_name, *cfg_file_name = NULL, *options = NULL;
>       UINTN gop_mode = ~0;
> -    EFI_SHIM_LOCK_PROTOCOL *shim_lock;
> +    SHIM_IMAGE_LOADER *shim_loader;
>       EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL;
>       union string section = { NULL }, name;
>       bool base_video = false;
> @@ -1590,12 +1593,24 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE 
> ImageHandle,
>        * device tree through the efi_check_dt_boot function, in this stage
>        * verify it.
>        */
> -    if ( kernel.ptr &&
> -         !kernel_verified &&
> -         !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
> -                                           (void **)&shim_lock)) &&
> -         (status = shim_lock->Verify(kernel.ptr, kernel.size)) != 
> EFI_SUCCESS )
> +    if ( kernel.ptr && !kernel_verified )
> +    {
>           PrintErrMesg(L"Dom0 kernel image could not be verified", status);

I think this PrintErrMesg() should be removed. It will print a bogus 
message (indeed it has not been verified *Yet*, but maybe it's going to be).
Also, PrintErrMesg calls blexit() which never returns, so the kernel 
verification with LoadImage will never happen and Xen boot will stop.

> +    status = efi_bs->LocateProtocol(&((EFI_GUID) SHIM_IMAGE_LOADER_GUID),
> +                                     NULL, (void **)&shim_loader);
> +    if ( EFI_ERROR(status) )
> +        PrintErrMesg(L"Error LocateProtocol IMAGE_LOADER", status);
> +
> +    if ( kernel.ptr ) {
> +        status = shim_loader->LoadImage(false, ImageHandle, NULL, (void 
> *)kernel.ptr, kernel.size, &loaded_kernel);
> +        if ( EFI_ERROR(status) )
> +            PrintErrMesg(L"LoadImage failed", status);
> +
> +        // LoadImage performs verification, now unload it to clean up
> +        status = shim_loader->UnloadImage(loaded_kernel);
> +        if ( EFI_ERROR(status) )
> +            PrintErrMesg(L"UnloadImage failed", status);
> +    }
>   
>       efi_arch_edd();
>   

Regards,

-- 


--
Yann Sionneau | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech