On Tue, Apr 01, 2025 at 11:26:43AM +0100, Mate Kukri wrote:
> Not reusing these handles will result in image measurements showing up
> twice in the event log.
>
> Signed-off-by: Mate Kukri <mate.ku...@canonical.com>
> ---
>  grub-core/kern/efi/sb.c | 16 ++++++++++++++++
>  include/grub/efi/sb.h   |  4 ++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
> index 34ba323ee..6c4dd5f85 100644
> --- a/grub-core/kern/efi/sb.c
> +++ b/grub-core/kern/efi/sb.c
> @@ -36,6 +36,8 @@ static grub_guid_t shim_loader_guid = 
> GRUB_EFI_SHIM_IMAGE_LOADER_GUID;
>  static grub_efi_loader_t *shim_loader = NULL;
>  static grub_efi_shim_lock_protocol_t *shim_lock = NULL;
>
> +static grub_efi_handle_t last_verified_image_handle;

static grub_efi_handle_t last_verified_image_handle = NULL;

>  /*
>   * Determine whether we're in secure boot mode.
>   *
> @@ -181,10 +183,16 @@ shim_lock_verifier_write (void *context __attribute__ 
> ((unused)), void *buf, gru
>
>    if (shim_loader)
>      {
> +      if (last_verified_image_handle)

if (last_verified_image_handle != NULL)

Please fix similar issues everywhere...

> +        {
> +          shim_loader->unload_image (last_verified_image_handle);
> +          last_verified_image_handle = NULL;

I am not entirely sure why you unload the image here. If it is really
needed I think it requires an explanation.

> +        }
>
>        if (shim_loader->load_image (false, grub_efi_image_handle, NULL, buf, 
> size, &image_handle) != GRUB_EFI_SUCCESS)
>          return grub_error (GRUB_ERR_BAD_SIGNATURE, N_("bad shim signature"));
>
> +      last_verified_image_handle = image_handle;
>        return GRUB_ERR_NONE;
>      }
>    if (shim_lock)
> @@ -242,3 +250,11 @@ grub_is_using_legacy_shim_lock_protocol (void)
>  {
>    return !shim_loader && shim_lock;
>  }
> +
> +grub_efi_handle_t
> +grub_efi_get_last_verified_image_handle (void)
> +{
> +  grub_efi_handle_t tmp = last_verified_image_handle;
> +  last_verified_image_handle = NULL;

Why do you reset last_verified_image_handle here? The "get" function
should not do that. And if it is needed then it has to be explained why.

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to