Re: [PATCH v8 10/10] efi: Disallow fallback to legacy Linux loader when shim says NX is required.

[Glenn Washburn]

Hi Daniel,

Looks like this and the two patches preceding this were not merged
into git, nor do I see discussion or hints that they would be dropped.
Is this intentional?

Glenn

On Wed,  9 Oct 2024 09:16:45 +0100
Mate Kukri <mate.kukri@canonical.com> wrote:

> Signed-off-by: Mate Kukri <mate.kukri@canonical.com>
> ---
>  grub-core/kern/efi/sb.c      | 28 ++++++++++++++++++++++++++++
>  grub-core/loader/efi/linux.c | 12 +++++++-----
>  include/grub/efi/api.h       |  2 ++
>  include/grub/efi/sb.h        |  2 ++
>  4 files changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
> index d3de39599..cbdb29ae1 100644
> --- a/grub-core/kern/efi/sb.c
> +++ b/grub-core/kern/efi/sb.c
> @@ -225,3 +225,31 @@ grub_shim_lock_verifier_setup (void)
>    grub_env_set ("shim_lock", "y");
>    grub_env_export ("shim_lock");
>  }
> +
> +bool
> +grub_efi_check_nx_required (void)
> +{
> +  int nx_required = 1; /* assume required, unless we can prove otherwise */
> +  grub_efi_status_t status;
> +  grub_size_t mok_policy_sz = 0;
> +  char *mok_policy = NULL;
> +  grub_uint32_t mok_policy_attrs = 0;
> +
> +  status = grub_efi_get_variable_with_attributes ("MokPolicy",
> +                                               &(grub_guid_t) 
> GRUB_EFI_SHIM_LOCK_GUID,
> +                                               &mok_policy_sz,
> +                                               (void **)&mok_policy,
> +                                               &mok_policy_attrs);
> +  if (status != GRUB_EFI_SUCCESS ||
> +      mok_policy_sz != 1 ||
> +      mok_policy == NULL ||
> +      mok_policy_attrs != GRUB_EFI_VARIABLE_BOOTSERVICE_ACCESS)
> +    goto out;
> +
> +  nx_required = !!(mok_policy[0] & GRUB_MOK_POLICY_NX_REQUIRED);
> +
> + out:
> +  grub_free (mok_policy);
> +
> +  return nx_required;
> +}
> diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c
> index d6860fdba..8760f2da9 100644
> --- a/grub-core/loader/efi/linux.c
> +++ b/grub-core/loader/efi/linux.c
> @@ -473,21 +473,23 @@ grub_cmd_linux (grub_command_t cmd __attribute__ 
> ((unused)),
>  
>    kernel_size = grub_file_size (file);
>  
> -  if (grub_arch_efi_linux_load_image_header (file, &lh) != GRUB_ERR_NONE)
>  #if !defined(__i386__) && !defined(__x86_64__)
> +  if (grub_arch_efi_linux_load_image_header (file, &lh) != GRUB_ERR_NONE)
>      goto fail;
>  #else
> -    goto fallback;
> -
> -  if (!initrd_use_loadfile2)
> +  if (grub_arch_efi_linux_load_image_header (file, &lh) != GRUB_ERR_NONE ||
> +      !initrd_use_loadfile2)
>      {
> +      /* We cannot use the legacy loader when NX is required */
> +      if (grub_efi_check_nx_required ())
> +        goto fail;
> +
>        /*
>         * This is a EFI stub image but it is too old to implement the 
> LoadFile2
>         * based initrd loading scheme, and Linux/x86 does not support the DT
>         * based method either. So fall back to the x86-specific loader that
>         * enters Linux in EFI mode but without going through its EFI stub.
>         */
> -fallback:
>        grub_file_close (file);
>        return grub_cmd_linux_x86_legacy (cmd, argc, argv);
>      }
> diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
> index 9ae908729..5771d96f2 100644
> --- a/include/grub/efi/api.h
> +++ b/include/grub/efi/api.h
> @@ -1785,6 +1785,8 @@ struct grub_efi_block_io
>  };
>  typedef struct grub_efi_block_io grub_efi_block_io_t;
>  
> +#define GRUB_MOK_POLICY_NX_REQUIRED  0x1
> +
>  struct grub_efi_shim_lock_protocol
>  {
>    /*
> diff --git a/include/grub/efi/sb.h b/include/grub/efi/sb.h
> index bf8d2db5f..be517b1dc 100644
> --- a/include/grub/efi/sb.h
> +++ b/include/grub/efi/sb.h
> @@ -33,6 +33,8 @@ EXPORT_FUNC (grub_efi_get_secureboot) (void);
>  
>  extern void
>  grub_shim_lock_verifier_setup (void);
> +extern bool
> +EXPORT_FUNC (grub_efi_check_nx_required) (void);
>  #else
>  static inline grub_uint8_t
>  grub_efi_get_secureboot (void)