>As already said on an earlier version, the use of !EFI_ERROR() here is a
>behavioral change from ...
>
>> @@ -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 )
>
>... checking for EFI_SUCCESS alone here. There's also nothing in the
>description justifying the change. (See the various EFI_WARN_* that exist.)
You're correct! The EFI_WARN_* responses should all be treated as failures,
I'll revert that particular change in the patch series
*Gerald Elder-Vass*
Senior Software Engineer
XenServer
Cambridge, UK
On Mon, Sep 8, 2025 at 9:56 AM Jan Beulich <jbeul...@suse.com> wrote:
> On 05.09.2025 14:10, Gerald Elder-Vass wrote:
> > @@ -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);
> > + }
> > +
> > + /* 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)) )
>
> As already said on an earlier version, the use of !EFI_ERROR() here is a
> behavioral change from ...
>
> > @@ -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 )
>
> ... checking for EFI_SUCCESS alone here. There's also nothing in the
> description justifying the change. (See the various EFI_WARN_* that exist.)
>
> Jan
>
