[U-Boot] [PATCH] efi_loader: device_path: allow for arbitrary length of file path

[Heinrich Schuchardt]

On 10/4/19 6:45 AM, AKASHI Takahiro wrote:
> This patch will lift the upper limit of maximum path length.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>

Thanks for addressing this issue.

Please, have a look at the side effects to our implementation of the
device path to text protocol. Here I assumed that a device node is at
most 512 bytes long. This will no longer hold true after your patch.

I think we should fix the device path to text protocol first before
merging this patch.

> ---
>   lib/efi_loader/efi_device_path.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 86297bb7c116..13a2b5e29db1 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -1017,8 +1017,7 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
>   	struct blk_desc *desc = NULL;
>   	disk_partition_t fs_partition;
>   	int part = 0;
> -	char filename[32] = { 0 }; /* dp->str is u16[32] long */
> -	char *s;
> +	char *filename, *s;

Below we are replacing slashes '/' by backslashes '\'. So shouldn't this
variable be called filepath?

>
>   	if (path && !file)
>   		return EFI_INVALID_PARAMETER;
> @@ -1042,13 +1041,16 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
>   	if (!path)
>   		return EFI_SUCCESS;
>
> -	snprintf(filename, sizeof(filename), "%s", path);
> +	filename = strdup(path);

Windows has a maximum file path length of 260 characters. Can we assume
in EFI that a path has a maximum length of 260 characters too?

At least we must ensure that the node length does not exceed 65535
bytes, the maximum possible value in the node length field.

Best regards

Heinrich

> +	if (!filename)
> +		return EFI_OUT_OF_RESOURCES;
>   	/* DOS style file path: */
>   	s = filename;
>   	while ((s = strchr(s, '/')))
>   		*s++ = '\\';
>   	*file = efi_dp_from_file(((!is_net && device) ? desc : NULL),
>   				 part, filename);
> +	free(filename);
>
>   	return EFI_SUCCESS;
>   }
>