1. Patchew
  2. Xen
  3. View series

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

Frediano Ziglio posted 1 patch 4 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250618184631.15489-1-frediano.ziglio@cloud.com
There is a newer version of this series
xen/common/efi/boot.c | 10 ++++++++++
1 file changed, 10 insertions(+)
[PATCH] xen/efi: Do not check kernel signature if it was embedded
Posted by Frediano Ziglio 4 months, 2 weeks ago 
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
Posted by Jan Beulich 4 months, 1 week ago 
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
Re: [PATCH] xen/efi: Do not check kernel signature if it was embedded
Posted by Marek Marczykowski-Górecki 4 months, 1 week ago 
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
Re: [PATCH] xen/efi: Do not check kernel signature if it was embedded
Posted by Frediano Ziglio 4 months, 1 week ago 
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
Posted by Marek Marczykowski-Górecki 4 months, 1 week ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.