Re: [PATCH] loader/i386/linux: report UEFI secure boot status to the Linux kernel

[Daniel Kiper]

On Tue, Oct 09, 2018 at 04:04:03PM +0000, Ignat Korchagin wrote:
> Linux kernel from 4.11 has secure_boot member as part of linux_kernel_params.
> Currently, GRUB does not populate it, so the kernel reports
> "Secure boot could not be determined" on boot. We can populate it in EFI mode,
> so the kernel "knows" the status.
>
> Signed-off-by: Ignat Korchagin <address@hidden>
> ---
>  grub-core/loader/i386/linux.c | 34 +++++++++++++++++++++++++++++++++-
>  include/grub/i386/linux.h     | 12 ++++++++++--
>  2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
> index 4eab55a2d..7fc188603 100644
> --- a/grub-core/loader/i386/linux.c
> +++ b/grub-core/loader/i386/linux.c
> @@ -396,6 +396,37 @@ grub_linux_boot_mmap_fill (grub_uint64_t addr, 
> grub_uint64_t size,
>    return 0;
>  }
>
> +#ifdef GRUB_MACHINE_EFI
> +static grub_uint8_t
> +grub_efi_secureboot_mode (void)
> +{
> +  grub_efi_guid_t efi_var_guid = GRUB_EFI_GLOBAL_VARIABLE_GUID;
> +  grub_size_t efi_var_size = 0;
> +  grub_uint8_t *secure_boot;
> +  grub_uint8_t *setup_mode;
> +  grub_uint8_t secureboot_mode = LINUX_EFI_SECUREBOOT_MODE_UNSET;
> +
> +  secure_boot = grub_efi_get_variable ("SecureBoot", &efi_var_guid, 
> &efi_var_size);
> +  setup_mode = grub_efi_get_variable ("SetupMode", &efi_var_guid, 
> &efi_var_size);
> +
> +  if (!secure_boot || !setup_mode)
> +    goto fail;
> +
> +  if ((*secure_boot == 0) || (*setup_mode == 1))
> +    secureboot_mode = LINUX_EFI_SECUREBOOT_MODE_DISABLED;
> +  else
> +    secureboot_mode = LINUX_EFI_SECUREBOOT_MODE_ENABLED;
> +
> +fail:
> +  if (setup_mode)
> +    grub_free (setup_mode);
> +  if (secure_boot)
> +    grub_free (secure_boot);
> +
> +  return secureboot_mode;

May I ask you to duplicate the logic from
linux/drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot()?
Additionally, please add the comment that it is taken from there.
And it is also worth mentioning the Linux kernel version or commit id.

> +}
> +#endif
> +
>  static grub_err_t
>  grub_linux_boot (void)
>  {
> @@ -574,6 +605,7 @@ grub_linux_boot (void)
>      grub_efi_uintn_t efi_desc_size;
>      grub_size_t efi_mmap_target;
>      grub_efi_uint32_t efi_desc_version;
> +    ctx.params->secure_boot = grub_efi_secureboot_mode ();
>      err = grub_efi_finish_boot_services (&efi_mmap_size, efi_mmap_buf, NULL,
>                                        &efi_desc_size, &efi_desc_version);
>      if (err)
> @@ -760,7 +792,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ 
> ((unused)),
>
>    linux_params.code32_start = prot_mode_target + lh.code32_start - 
> GRUB_LINUX_BZIMAGE_ADDR;
>    linux_params.kernel_alignment = (1 << align);
> -  linux_params.ps_mouse = linux_params.padding10 =  0;
> +  linux_params.ps_mouse = linux_params.padding11 =  0;
>
>    len = sizeof (linux_params) - sizeof (lh);
>    if (grub_file_read (file, (char *) &linux_params + sizeof (lh), len) != 
> len)
> diff --git a/include/grub/i386/linux.h b/include/grub/i386/linux.h
> index 60c7c3b5e..4493a3fdb 100644
> --- a/include/grub/i386/linux.h
> +++ b/include/grub/i386/linux.h
> @@ -270,7 +270,15 @@ struct linux_kernel_params
>
>    grub_uint8_t mmap_size;            /* 1e8 */
>
> -  grub_uint8_t padding9[0x1f1 - 0x1e9];
> +  grub_uint8_t padding9[0x1ec - 0x1e9];
> +
> +  grub_uint8_t secure_boot;             /* 1ec */
> +#define LINUX_EFI_SECUREBOOT_MODE_UNSET    0
> +#define LINUX_EFI_SECUREBOOT_MODE_UNKNOWN  1
> +#define LINUX_EFI_SECUREBOOT_MODE_DISABLED 2
> +#define LINUX_EFI_SECUREBOOT_MODE_ENABLED  3

Please mov this to constants section above.

Daniel