On Fri, Sep 05, 2025 at 10:05:32AM +0000, Gerald Elder-Vass wrote: > The existing Verify functionality of the Shim lock protocol is > deprecated and will be removed, the alternative it to 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. > > If the LoadImage protocol isn't available then fall back to the Shim > Lock (Verify) interface. > > Log when the kernel is not verified and fail if this occurs > when secure boot mode is enabled. > > Signed-off-by: Gerald Elder-Vass <[email protected]> > Signed-off-by: Kevin Lampis <[email protected]> > --- > CC: Marek Marczykowski-Górecki <[email protected]> > CC: "Daniel P. Smith" <[email protected]> > CC: Jan Beulich <[email protected]> > CC: Andrew Cooper <[email protected]> > CC: Anthony PERARD <[email protected]> > CC: Michal Orzel <[email protected]> > CC: Julien Grall <[email protected]> > CC: "Roger Pau Monné" <[email protected]> > CC: Stefano Stabellini <[email protected]> > > v3: > - Use Shim Image by default, fall back to Shim Lock > --- > xen/common/efi/boot.c | 59 +++++++++++++++++++++++++++++++++++++------ > 1 file changed, 51 insertions(+), 8 deletions(-) > > diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c > index e7e3dffa7ddc..1f63473d264d 100644 > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -38,6 +38,8 @@ > { 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 \ > @@ -70,6 +72,13 @@ 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; > > typedef EFI_STATUS > @@ -1047,6 +1056,46 @@ static UINTN __init > efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, > return gop_mode; > } > > +static void __init efi_verify_kernel(EFI_HANDLE ImageHandle) > +{ > + static EFI_GUID __initdata shim_image_guid = SHIM_IMAGE_LOADER_GUID; > + static EFI_GUID __initdata shim_lock_guid = SHIM_LOCK_PROTOCOL_GUID; > + SHIM_IMAGE_LOADER *shim_loader; > + EFI_HANDLE loaded_kernel; > + EFI_SHIM_LOCK_PROTOCOL *shim_lock; > + EFI_STATUS status; > + bool verified = false; > + > + /* Look for LoadImage first */ > + if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_image_guid, NULL, > + (void **)&shim_loader)) ) > + { > + status = shim_loader->LoadImage(false, ImageHandle, NULL, > + (void *)kernel.ptr, kernel.size, > + &loaded_kernel); > + if ( !EFI_ERROR(status) ) > + verified = true; > + > + /* LoadImage performed verification, now clean up with UnloadImage */ > + shim_loader->UnloadImage(loaded_kernel);
Is UnloadImage really appropriate even if LoadImage failed?
> + }
> +
> + /* else fall back to Shim Lock */
> + if ( !verified &&
> + !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
> + (void **)&shim_lock)) &&
> + !EFI_ERROR(shim_lock->Verify(kernel.ptr, kernel.size)) )
> + verified = true;
> +
> + if ( !verified )
> + {
> + PrintStr(L"Kernel was not verified\n");
> +
> + if ( efi_secure_boot )
> + blexit(L"Failed to verify kernel");
Better be more explicit why it's fatal, like "Refusing to boot
unverified kernel with UEFI SecureBoot enabled".
> + }
> +}
> +
> static void __init efi_tables(void)
> {
> unsigned int i;
> @@ -1334,13 +1383,11 @@ 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;
> unsigned int i;
> CHAR16 *file_name, *cfg_file_name = NULL, *options = NULL;
> UINTN gop_mode = ~0;
> - EFI_SHIM_LOCK_PROTOCOL *shim_lock;
> EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL;
> union string section = { NULL }, name;
> bool base_video = false;
> @@ -1591,12 +1638,8 @@ 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 )
> - PrintErrMesg(L"Dom0 kernel image could not be verified", status);
> + if ( kernel.ptr && !kernel_verified )
> + efi_verify_kernel(ImageHandle);
>
> efi_arch_edd();
>
> --
> 2.47.3
>
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
signature.asc
Description: PGP signature
