Re: [PATCH] efi/chainloader: fix device path handed to loaded image

[Oliver Steffen]

According to UEFI Specification Version 2.9, Section 10.3.5
multiple File nodes are allowed in the path.  So this is not really
a bug.
I suspect the problem is at the consumer side where the string is generated.

Thanks,
Oliver

On Fri, Sep 30, 2022 at 4:51 PM Oliver Steffen <osteffen@redhat.com> wrote:
>
> Do not split the path on the filesystem into directory
> and filename, and do not add them as two separate device
> path nodes.  Instead add one node for the full path.
>
> When chain loading an efi binary the path constructed
> currently is, for example:
> /ACPI(a0341d0,0)/PCI(2,1f)/Sata(0,ffff,0)/File(\efi\Linux)/File(vmlinuz.efi)/EndEntire
> This is interpreted by the firmware as:
> PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/\efi\Linux/vmlinuz.efi
> Which is invalid because it contains a / where a \ belongs.
> This / is in fact the device path node separator, and is not part of
> any File() path node.
> Using one node for the full path on the filesystem results in:
> PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/\efi\Linux\vmlinuz.efi
>
> Signed-off-by: Oliver Steffen <osteffen@redhat.com>
> ---
>  grub-core/loader/efi/chainloader.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/grub-core/loader/efi/chainloader.c 
> b/grub-core/loader/efi/chainloader.c
> index 7557eb269..6276cf491 100644
> --- a/grub-core/loader/efi/chainloader.c
> +++ b/grub-core/loader/efi/chainloader.c
> @@ -170,7 +170,6 @@ make_file_path (grub_efi_device_path_t *dp, const char 
> *filename)
>      }
>
>    /* File Path is NULL terminated. Allocate space for 2 extra characters */
> -  /* FIXME why we split path in two components? */
>    file_path = grub_malloc (size
>                            + ((grub_strlen (dir_start) + 2)
>                               * GRUB_MAX_UTF16_PER_UTF8
> @@ -186,19 +185,12 @@ make_file_path (grub_efi_device_path_t *dp, const char 
> *filename)
>                                   + ((char *) d - (char *) dp));
>    grub_efi_print_device_path (d);
>    if (copy_file_path ((grub_efi_file_path_device_path_t *) d,
> -                     dir_start, dir_end - dir_start) != GRUB_ERR_NONE)
> +                     dir_start, grub_strlen(dir_start)) != GRUB_ERR_NONE)
>      {
> - fail:
>        grub_free (file_path);
>        return 0;
>      }
>
> -  /* Fill the file path for the file.  */
> -  d = GRUB_EFI_NEXT_DEVICE_PATH (d);
> -  if (copy_file_path ((grub_efi_file_path_device_path_t *) d,
> -                     dir_end + 1, grub_strlen (dir_end + 1)) != 
> GRUB_ERR_NONE)
> -    goto fail;
> -
>    /* Fill the end of device path nodes.  */
>    d = GRUB_EFI_NEXT_DEVICE_PATH (d);
>    d->type = GRUB_EFI_END_DEVICE_PATH_TYPE;
> --
> 2.37.3
>