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

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

[Frediano Ziglio]

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.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
 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

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

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 2262 bytes --]

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.
 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> ---
>  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Frediano Ziglio]

On Thu, Jun 19, 2025 at 1:17 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> 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@cloud.com>
> > ---
> >  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

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

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 2930 bytes --]

On Thu, Jun 19, 2025 at 01:33:20PM +0100, Frediano Ziglio wrote:
> On Thu, Jun 19, 2025 at 1:17 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> 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?

Yes, and reword the code comment a bit.

> Beside that sentence, any issue with this change?

Other than that it looks fine for me.

> > > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> > > ---
> > >  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

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Jan Beulich]

On 18.06.2025 20:46, Frediano Ziglio wrote:
> --- 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;

May I suggest to drop the "was" infix from the variable name? The name is imo
as clear without it.

Jan