Re: [PATCH 1/2] efi/chainloader: Do not require a valid $root when chainloading

[Dimitri John Ledkov]

Hi,

This is interesting. I had to work around this same issue in loopback
to allow chainloading from loopback devices see
https://github.com/rhboot/grub2/commit/0e5cb733f3cb227293ea58397ea10891519095f0



On Fri, 26 Aug 2022 at 05:34, Glenn Washburn
<development@efficientek.com> wrote:
>
> The EFI chainloader checks that a device path can be created for the $root
> device before allowing chainloading to a given file. This is probably to
> ensure that the given file can be accessed and loaded by the firmware.
> However, since GRUB is loading the image itself, the firmware need not
> be able to access the file location of the image. So remove this check.
>
> Also, this fixes an issue where chainloading an image file on a location
> that is accessible by the firmware, eg. (hd0,1)/efi/boot.efi, would
> fail when root is a location inaccessible by the firmware, eg. memdisk.
>
> Use GRUB_EFI_BYTES_TO_PAGES() instead of donig the calculation explicitly.
>
> Add comment noting the section where the load options for the chainloaded
> EFI application is constructed.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/loader/efi/chainloader.c | 31 +++++++++++-------------------
>  1 file changed, 11 insertions(+), 20 deletions(-)
>
> diff --git a/grub-core/loader/efi/chainloader.c 
> b/grub-core/loader/efi/chainloader.c
> index 7557eb269b..5aac3c59fd 100644
> --- a/grub-core/loader/efi/chainloader.c
> +++ b/grub-core/loader/efi/chainloader.c
> @@ -32,6 +32,7 @@
>  #include <grub/efi/api.h>
>  #include <grub/efi/efi.h>
>  #include <grub/efi/disk.h>
> +#include <grub/efi/memory.h>
>  #include <grub/command.h>
>  #include <grub/i18n.h>
>  #include <grub/net.h>
> @@ -239,11 +240,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>    if (! file)
>      goto fail;
>
> -  /* Get the root device's device path.  */
> -  dev = grub_device_open (0);
> -  if (! dev)
> -    goto fail;
> -
> +  dev = file->device;
>    if (dev->disk)
>      dev_handle = grub_efidisk_get_device_handle (dev->disk);
>    else if (dev->net && dev->net->server)
> @@ -267,18 +264,15 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>    if (dev_handle)
>      dp = grub_efi_get_device_path (dev_handle);
>
> -  if (! dp)
> +  if (dp != NULL)
>      {
> -      grub_error (GRUB_ERR_BAD_DEVICE, "not a valid root device");
> -      goto fail;
> -    }
> -
> -  file_path = make_file_path (dp, filename);
> -  if (! file_path)
> -    goto fail;
> +      file_path = make_file_path (dp, filename);
> +      if (file_path == NULL)
> +       goto fail;
>
> -  grub_printf ("file path: ");
> -  grub_efi_print_device_path (file_path);
> +      grub_printf ("file path: ");
> +      grub_efi_print_device_path (file_path);
> +    }
>
>    size = grub_file_size (file);
>    if (!size)
> @@ -287,7 +281,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>                   filename);
>        goto fail;
>      }
> -  pages = (((grub_efi_uintn_t) size + ((1 << 12) - 1)) >> 12);
> +  pages = (grub_efi_uintn_t) GRUB_EFI_BYTES_TO_PAGES (size);
>
>    status = efi_call_4 (b->allocate_pages, GRUB_EFI_ALLOCATE_ANY_PAGES,
>                               GRUB_EFI_LOADER_CODE,
> @@ -370,6 +364,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>      }
>    loaded_image->device_handle = dev_handle;
>
> +  /* Build load options with arguments from chainloader command line. */
>    if (argc > 1)
>      {
>        int i, len;
> @@ -400,7 +395,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>      }
>
>    grub_file_close (file);
> -  grub_device_close (dev);
>
>    /* We're finished with the source image buffer and file path now. */
>    efi_call_2 (b->free_pages, address, pages);
> @@ -411,9 +405,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>
>   fail:
>
> -  if (dev)
> -    grub_device_close (dev);
> -
>    if (file)
>      grub_file_close (file);
>
> --
> 2.34.1
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
okurrr,

Dimitri