Re: [PATCH] xen/efi: Do not check kernel signature if it was embedded

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Thu, Jun 19, 2025 at 1:17 PM Marek Marczykowski-Górecki
<marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Jun 18, 2025 at 07:46:28PM +0100, Frediano Ziglio wrote:
> > Using UKI it's possible to embed Linux kernel into xen.efi file.
> > In this case the signature for Secure Boot is applied to the
> > whole xen.efi, including the kernel.
> > So checking for specific signature for the kernel is not
> > needed.
> > In case Secure Boot is not enabled there's no reason to check
> > kernel signature.
>
> The last sentence (here and in the comment below) seem to be unrelated
> to this change - it's more about shim lock protocol being available,
> which this patch doesn't change.
>

Should I just remove the sentence?
Beside that sentence, any issue with this change?

> > Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> > ---
> >  xen/common/efi/boot.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> > index e39fbc3529..7077af3f5d 100644
> > --- a/xen/common/efi/boot.c
> > +++ b/xen/common/efi/boot.c
> > @@ -1291,6 +1291,7 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE 
> > ImageHandle,
> >      bool base_video = false;
> >      const char *option_str;
> >      bool use_cfg_file;
> > +    bool kernel_was_verified = false;
> >      int dt_modules_found;
> >
> >      __set_bit(EFI_BOOT, &efi_flags);
> > @@ -1461,6 +1462,14 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE 
> > ImageHandle,
> >              read_file(dir_handle, s2w(&name), &kernel, option_str);
> >              efi_bs->FreePool(name.w);
> >          }
> > +        else
> > +        {
> > +            /*
> > +             * As kernel was embedded it was either verified for Secure 
> > Boot
> > +             * or Secure Boot is not enabled.
> > +             */
> > +            kernel_was_verified = true;
> > +        }
> >
> >          if ( !read_section(loaded_image, L"ramdisk", &ramdisk, NULL) )
> >          {
> > @@ -1534,6 +1543,7 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE 
> > ImageHandle,
> >       * verify it.
> >       */
> >      if ( kernel.ptr &&
> > +         !kernel_was_verified &&
> >           !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
> >                                             (void **)&shim_lock)) &&
> >           (status = shim_lock->Verify(kernel.ptr, kernel.size)) != 
> > EFI_SUCCESS )
> > --
> > 2.43.0
> >
>
> --
> Best Regards,
> Marek Marczykowski-Górecki
> Invisible Things Lab