[PATCH RH grub2] arm64: Fix EFI loader kernel image allocation

[Ard Biesheuvel]

On Mon, 26 Jul 2021 at 10:21, Benjamin Herrenschmidt
<benh at kernel.crashing.org> wrote:
>
> We are currently allocating just enough memory for the file size,
> which means that the kernel BSS is in limbo (and not even zeroed).
>
> Instead, use the PE optional header in which the kernel puts the
> actual size it needs, including BSS, and make sure we clear it.
>
> Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> ---
>
> This bug comes with the original shim_lock patch which removes LoadImage/StartImage
> and directly calls into the loaded image instead. It has been observbed to
> occasionally cause boot failures on EC2 ARM instances.
>
> I know at least Fedora/RH (and thus Amazon Linux 2) and Ubuntu are affected.
>

Thanks for fixing this. Does GRUB at least honor the minimum image
alignment described in the PE/COFF header?

Acked-by: Ard Biesheuvel <ardb at kernel.org>

>  grub-core/loader/arm64/linux.c | 82 ++++++++++++++++++++++------------
>  1 file changed, 54 insertions(+), 28 deletions(-)
>
> diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> index 47f8cf0d8..f568d452d 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -311,14 +311,31 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
>    return grub_errno;
>  }
>
> +static grub_err_t
> +parse_pe_header (void *kernel, grub_uint64_t *total_size, grub_uint32_t *entry_offset)
> +{
> +  struct linux_arch_kernel_header *lh = kernel;
> +  struct grub_armxx_linux_pe_header *pe;
> +
> +  pe = (void *)((unsigned long)kernel + lh->hdr_offset);
> +
> +  if (pe->opt.magic != GRUB_PE32_PE64_MAGIC)
> +    return grub_error(GRUB_ERR_BAD_OS, "Invalid PE optional header magic");
> +
> +  *total_size   = pe->opt.image_size;
> +  *entry_offset = pe->opt.entry_addr;
> +
> +  return GRUB_ERR_NONE;
> +}
> +
>  static grub_err_t
>  grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
>                 int argc, char *argv[])
>  {
>    grub_file_t file = 0;
> -  struct linux_arch_kernel_header lh;
> -  struct grub_armxx_linux_pe_header *pe;
>    grub_err_t err;
> +  grub_uint64_t filelen;
> +  void *kernel = NULL;
>    int rc;
>
>    grub_dl_ref (my_mod);
> @@ -333,40 +350,24 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
>    if (!file)
>      goto fail;
>
> -  kernel_size = grub_file_size (file);
> -
> -  if (grub_file_read (file, &lh, sizeof (lh)) < (long) sizeof (lh))
> -    return grub_errno;
> -
> -  if (grub_arch_efi_linux_check_image (&lh) != GRUB_ERR_NONE)
> -    goto fail;
> -
> -  grub_loader_unset();
> -
> -  grub_dprintf ("linux", "kernel file size: %lld\n", (long long) kernel_size);
> -  kernel_addr = grub_efi_allocate_any_pages (GRUB_EFI_BYTES_TO_PAGES (kernel_size));
> -  grub_dprintf ("linux", "kernel numpages: %lld\n",
> -               (long long) GRUB_EFI_BYTES_TO_PAGES (kernel_size));
> -  if (!kernel_addr)
> +  filelen = grub_file_size (file);
> +  kernel = grub_malloc(filelen);
> +  if (!kernel)
>      {
> -      grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +      grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("cannot allocate kernel load buffer"));
>        goto fail;
>      }
>
> -  grub_file_seek (file, 0);
> -  if (grub_file_read (file, kernel_addr, kernel_size)
> -      < (grub_int64_t) kernel_size)
> +  if (grub_file_read (file, kernel, filelen) < filelen)
>      {
> -      if (!grub_errno)
> -       grub_error (GRUB_ERR_BAD_OS, N_("premature end of file %s"), argv[0]);
> +      grub_error (GRUB_ERR_FILE_READ_ERROR, N_("Can't read kernel %s"),
> +                 argv[0]);
>        goto fail;
>      }
>
> -  grub_dprintf ("linux", "kernel @ %p\n", kernel_addr);
> -
>    if (grub_efi_get_secureboot () == GRUB_EFI_SECUREBOOT_MODE_ENABLED)
>      {
> -      rc = grub_linuxefi_secure_validate (kernel_addr, kernel_size);
> +      rc = grub_linuxefi_secure_validate (kernel, filelen);
>        if (rc <= 0)
>         {
>           grub_error (GRUB_ERR_INVALID_COMMAND,
> @@ -375,8 +376,30 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
>         }
>      }
>
> -  pe = (void *)((unsigned long)kernel_addr + lh.hdr_offset);
> -  handover_offset = pe->opt.entry_addr;
> +  if (grub_arch_efi_linux_check_image (kernel) != GRUB_ERR_NONE)
> +    goto fail;
> +  if (parse_pe_header (kernel, &kernel_size, &handover_offset) != GRUB_ERR_NONE)
> +    goto fail;
> +  grub_dprintf ("linux", "kernel mem size     : %lld\n", (long long) kernel_size);
> +  grub_dprintf ("linux", "kernel entry offset : %d\n", handover_offset);
> +
> +  grub_loader_unset();
> +
> +  kernel_addr = grub_efi_allocate_any_pages (GRUB_EFI_BYTES_TO_PAGES (kernel_size));
> +  grub_dprintf ("linux", "kernel numpages: %lld\n",
> +               (long long) GRUB_EFI_BYTES_TO_PAGES (kernel_size));
> +  if (!kernel_addr)
> +    {
> +      grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +      goto fail;
> +    }
> +
> +  grub_dprintf ("linux", "kernel @ %p\n", kernel_addr);
> +  grub_memcpy (kernel_addr, kernel, grub_min(filelen, kernel_size));
> +  if (kernel_size > filelen)
> +    grub_memset ((char *)kernel_addr + filelen, 0, kernel_size - filelen);
> +  grub_free(kernel);
> +  kernel = NULL;
>
>    cmdline_size = grub_loader_cmdline_size (argc, argv) + sizeof (LINUX_IMAGE);
>    linux_args = grub_malloc (cmdline_size);
> @@ -400,6 +423,9 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
>      }
>
>  fail:
> +  if (kernel)
> +    grub_free (kernel);
> +
>    if (file)
>      grub_file_close (file);
>
>
>
>
> _______________________________________________
> Efi mailing list
> Efi at lists.einval.com
> https://lists.einval.com/cgi-bin/mailman/listinfo/efi