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

[AKASHI Takahiro]

On Fri, Oct 04, 2019 at 08:09:57AM +0200, Heinrich Schuchardt wrote:
> 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.

Whether my patch is applied or not, we may possibly hit this limitation.

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

Anyhow, I will check.

> >---
> >  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?

Okay.

> >
> >  	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?

Why do you stick to Windows here?
In my patch of "install FILE_SYSTEM_PROTOCOL to whole disk,"
you wanted to allow file systems other than FAT.

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

Okay.

-Takahiro Akashi

> 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;
> >  }
> >
>