[U-Boot] [PATCH v2 0/3] efi_loader: device path: handle a long file path name

efi_dp_from_name() uses a fixed length (32) of buffer, and so it cannot handle a long file path name. This patch set lifts the upper limit as well as other limitations regarding file paths.
For example, without this patch set, => efi boot add 1 TEST scsi 1:1 /0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef/Shell.efi "" => efi boot dump Boot0001: attributes: A-- (0x00000001) label: TEST file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(1,MBR,0x086246ba,0x800,0x40000)/\0123456789abcdef0123456789abcd data: 00000000: 00 00
=> The path was truncated
With this patch set applied, => efi boot add 1 TEST scsi 1:1 /0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef/Shell.efi "" => efi boot dump Boot0001: attributes: A-- (0x00000001) label: TEST file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(1,MBR,0x086246ba,0x800,0x40000)/\0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef\0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789 data: 00000000: 00 00
=> still truncated, but it was due to limitation of command line processing (CONFIG_SYS_CBSIZE?), not due to device path
Changes in v2 (Oct 9, 2019) * add patch#1 after Heinrich's comment * add patch#2 after Heinrich's comment
AKASHI Takahiro (3): efi_loader: device_path: check against file path length efi_loader: device_path: lift the upper limit in dp-to-text conversion efi_loader: device_path: allow for arbitrary length of file path
lib/efi_loader/efi_device_path.c | 23 ++++-- lib/efi_loader/efi_device_path_to_text.c | 90 +++++++++++++++++++++--- 2 files changed, 96 insertions(+), 17 deletions(-)

device_path strcuture has 2 bytes of "length" field, and so file path length should not exceed this limit, 65535.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- lib/efi_loader/efi_device_path.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 86297bb7c116..9f772fc924fb 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -14,6 +14,7 @@ #include <part.h> #include <sandboxblockdev.h> #include <asm-generic/unaligned.h> +#include <linux/compat.h> /* U16_MAX */
#ifdef CONFIG_SANDBOX const efi_guid_t efi_guid_host_dev = U_BOOT_HOST_DEV_GUID; @@ -868,13 +869,16 @@ struct efi_device_path *efi_dp_from_file(struct blk_desc *desc, int part, { struct efi_device_path_file_path *fp; void *buf, *start; - unsigned dpsize = 0, fpsize; + size_t dpsize = 0, fpsize;
if (desc) dpsize = dp_part_size(desc, part);
fpsize = sizeof(struct efi_device_path) + 2 * (utf8_utf16_strlen(path) + 1); + if (fpsize > U16_MAX) + return NULL; + dpsize += fpsize;
start = buf = dp_alloc(dpsize + sizeof(END)); @@ -888,7 +892,7 @@ struct efi_device_path *efi_dp_from_file(struct blk_desc *desc, int part, fp = buf; fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE; fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH; - fp->dp.length = fpsize; + fp->dp.length = (u16)fpsize; path_to_uefi(fp->str, path); buf += fpsize;
@@ -1050,5 +1054,8 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr, *file = efi_dp_from_file(((!is_net && device) ? desc : NULL), part, filename);
+ if (!file) + return EFI_INVALID_PARAMETER; + return EFI_SUCCESS; }

On 10/9/19 9:19 AM, AKASHI Takahiro wrote:
device_path strcuture has 2 bytes of "length" field, and so file path length should not exceed this limit, 65535.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Thanks for the patch.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de

There is no practical reason to set a maxmum length of text either for file path or whole device path in device path-to-text conversion.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- lib/efi_loader/efi_device_path_to_text.c | 90 +++++++++++++++++++++--- 1 file changed, 80 insertions(+), 10 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index b158641e3043..75aabe66bf77 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -7,12 +7,12 @@
#include <common.h> #include <efi_loader.h> +#include <malloc.h>
#define MAC_OUTPUT_LEN 22 #define UNKNOWN_OUTPUT_LEN 23
#define MAX_NODE_LEN 512 -#define MAX_PATH_LEN 1024
const efi_guid_t efi_guid_device_path_to_text_protocol = EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID; @@ -228,8 +228,7 @@ static char *dp_media(char *s, struct efi_device_path *dp) struct efi_device_path_file_path *fp = (struct efi_device_path_file_path *)dp; int slen = (dp->length - sizeof(*dp)) / 2; - if (slen > MAX_NODE_LEN - 2) - slen = MAX_NODE_LEN - 2; + s += sprintf(s, "%-.*ls", slen, fp->str); break; } @@ -240,6 +239,31 @@ static char *dp_media(char *s, struct efi_device_path *dp) return s; }
+/* + * Return a maximum size of buffer to be allocated for conversion + * + * @dp device path or node + * @return maximum size of buffer + */ +static size_t efi_dp_text_max(struct efi_device_path *dp) +{ + /* + * We don't have be very accurate here. + * Currently, there are only two cases where a maximum size + * of buffer may go over MAX_NODE_LEN. + */ + if (dp->type == DEVICE_PATH_TYPE_HARDWARE_DEVICE && + dp->sub_type == DEVICE_PATH_SUB_TYPE_VENDOR) + /* VenHw(<guid>, xxxxxxxx...) */ + return 8 + 36 + (dp->length - sizeof(*dp)) * 2 + 1; + + if (dp->type == DEVICE_PATH_TYPE_MEDIA_DEVICE && + dp->sub_type == DEVICE_PATH_SUB_TYPE_FILE_PATH) + return dp->length - sizeof(*dp) + 2 /* u16 NULL */; + + return MAX_NODE_LEN; +} + /* * Converts a single node to a char string. * @@ -293,16 +317,27 @@ static uint16_t EFIAPI *efi_convert_device_node_to_text( bool display_only, bool allow_shortcuts) { - char str[MAX_NODE_LEN]; + char *str; + size_t str_size; uint16_t *text = NULL;
EFI_ENTRY("%p, %d, %d", device_node, display_only, allow_shortcuts);
if (!device_node) goto out; + + str_size = efi_dp_text_max(device_node); + if (!str_size) + goto out; + + str = malloc(str_size); + if (!str) + goto out; + efi_convert_single_device_node_to_text(str, device_node);
text = efi_str_to_u16(str); + free(str);
out: EFI_EXIT(EFI_SUCCESS); @@ -327,24 +362,59 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text( bool allow_shortcuts) { uint16_t *text = NULL; - char buffer[MAX_PATH_LEN]; - char *str = buffer; + char *buffer = NULL, *str; + size_t buf_size, buf_left; + efi_status_t ret = EFI_SUCCESS;
EFI_ENTRY("%p, %d, %d", device_path, display_only, allow_shortcuts);
- if (!device_path) + if (!device_path) { + ret = EFI_INVALID_PARAMETER; goto out; - while (device_path && - str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) { + } + + buf_size = 1024; + buf_left = buf_size; + buffer = malloc(buf_size); + if (!buffer) { + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + + str = buffer; + while (device_path) { + char *buf_new; + size_t buf_new_size, dp_text_len; + *str++ = '/'; + buf_left--; + dp_text_len = efi_dp_text_max(device_path); + if (buf_left < dp_text_len) { + buf_new_size = buf_size + dp_text_len; + buf_new = malloc(buf_new_size); + if (!buf_new) { + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + + memcpy(buf_new, buffer, str - buffer); + buf_left = buf_new_size - (str - buffer); + str = buf_new + (str - buffer); + free(buffer); + buffer = buf_new; + } + str = efi_convert_single_device_node_to_text(str, device_path); + device_path = efi_dp_next(device_path); }
text = efi_str_to_u16(buffer);
out: - EFI_EXIT(EFI_SUCCESS); + free(buffer); + EFI_EXIT(ret); + return text; }

On 10/9/19 9:19 AM, AKASHI Takahiro wrote:
There is no practical reason to set a maxmum length of text either for file path or whole device path in device path-to-text conversion.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_device_path_to_text.c | 90 +++++++++++++++++++++--- 1 file changed, 80 insertions(+), 10 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index b158641e3043..75aabe66bf77 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -7,12 +7,12 @@
#include <common.h> #include <efi_loader.h> +#include <malloc.h>
#define MAC_OUTPUT_LEN 22 #define UNKNOWN_OUTPUT_LEN 23
#define MAX_NODE_LEN 512 -#define MAX_PATH_LEN 1024
const efi_guid_t efi_guid_device_path_to_text_protocol = EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID; @@ -228,8 +228,7 @@ static char *dp_media(char *s, struct efi_device_path *dp) struct efi_device_path_file_path *fp = (struct efi_device_path_file_path *)dp; int slen = (dp->length - sizeof(*dp)) / 2;
if (slen > MAX_NODE_LEN - 2)
slen = MAX_NODE_LEN - 2;
- s += sprintf(s, "%-.*ls", slen, fp->str); break; }
@@ -240,6 +239,31 @@ static char *dp_media(char *s, struct efi_device_path *dp) return s; }
+/*
- Return a maximum size of buffer to be allocated for conversion
- @dp device path or node
- @return maximum size of buffer
- */
+static size_t efi_dp_text_max(struct efi_device_path *dp) +{
- /*
* We don't have be very accurate here.
* Currently, there are only two cases where a maximum size
* of buffer may go over MAX_NODE_LEN.
*/
- if (dp->type == DEVICE_PATH_TYPE_HARDWARE_DEVICE &&
dp->sub_type == DEVICE_PATH_SUB_TYPE_VENDOR)
/* VenHw(<guid>, xxxxxxxx...) */
return 8 + 36 + (dp->length - sizeof(*dp)) * 2 + 1;
Isn't this off by factor 2?
L"VenHw(,)" is 16 bytes long. L"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxxxxxx" is 72 bytes long Each L"xx" is 4 bytes long. The terminating L'\0' is 2 bytes long. The length of u16[] cannot be odd.
Best regards
Heinrich
- if (dp->type == DEVICE_PATH_TYPE_MEDIA_DEVICE &&
dp->sub_type == DEVICE_PATH_SUB_TYPE_FILE_PATH)
return dp->length - sizeof(*dp) + 2 /* u16 NULL */;
- return MAX_NODE_LEN;
+}
- /*
- Converts a single node to a char string.
@@ -293,16 +317,27 @@ static uint16_t EFIAPI *efi_convert_device_node_to_text( bool display_only, bool allow_shortcuts) {
- char str[MAX_NODE_LEN];
char *str;
size_t str_size; uint16_t *text = NULL;
EFI_ENTRY("%p, %d, %d", device_node, display_only, allow_shortcuts);
if (!device_node) goto out;
str_size = efi_dp_text_max(device_node);
if (!str_size)
goto out;
str = malloc(str_size);
if (!str)
goto out;
efi_convert_single_device_node_to_text(str, device_node);
text = efi_str_to_u16(str);
free(str);
out: EFI_EXIT(EFI_SUCCESS);
@@ -327,24 +362,59 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text( bool allow_shortcuts) { uint16_t *text = NULL;
- char buffer[MAX_PATH_LEN];
- char *str = buffer;
char *buffer = NULL, *str;
size_t buf_size, buf_left;
efi_status_t ret = EFI_SUCCESS;
EFI_ENTRY("%p, %d, %d", device_path, display_only, allow_shortcuts);
- if (!device_path)
- if (!device_path) {
goto out;ret = EFI_INVALID_PARAMETER;
- while (device_path &&
str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
}
buf_size = 1024;
buf_left = buf_size;
buffer = malloc(buf_size);
if (!buffer) {
ret = EFI_OUT_OF_RESOURCES;
goto out;
}
str = buffer;
while (device_path) {
char *buf_new;
size_t buf_new_size, dp_text_len;
*str++ = '/';
buf_left--;
dp_text_len = efi_dp_text_max(device_path);
if (buf_left < dp_text_len) {
buf_new_size = buf_size + dp_text_len;
buf_new = malloc(buf_new_size);
if (!buf_new) {
ret = EFI_OUT_OF_RESOURCES;
goto out;
}
memcpy(buf_new, buffer, str - buffer);
buf_left = buf_new_size - (str - buffer);
str = buf_new + (str - buffer);
free(buffer);
buffer = buf_new;
}
str = efi_convert_single_device_node_to_text(str, device_path);
device_path = efi_dp_next(device_path); }
text = efi_str_to_u16(buffer);
out:
- EFI_EXIT(EFI_SUCCESS);
- free(buffer);
- EFI_EXIT(ret);
- return text; }

On Wed, Oct 09, 2019 at 07:41:40PM +0200, Heinrich Schuchardt wrote:
On 10/9/19 9:19 AM, AKASHI Takahiro wrote:
There is no practical reason to set a maxmum length of text either for file path or whole device path in device path-to-text conversion.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_device_path_to_text.c | 90 +++++++++++++++++++++--- 1 file changed, 80 insertions(+), 10 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index b158641e3043..75aabe66bf77 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -7,12 +7,12 @@
#include <common.h> #include <efi_loader.h> +#include <malloc.h>
#define MAC_OUTPUT_LEN 22 #define UNKNOWN_OUTPUT_LEN 23
#define MAX_NODE_LEN 512 -#define MAX_PATH_LEN 1024
const efi_guid_t efi_guid_device_path_to_text_protocol = EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID; @@ -228,8 +228,7 @@ static char *dp_media(char *s, struct efi_device_path *dp) struct efi_device_path_file_path *fp = (struct efi_device_path_file_path *)dp; int slen = (dp->length - sizeof(*dp)) / 2;
if (slen > MAX_NODE_LEN - 2)
slen = MAX_NODE_LEN - 2;
- s += sprintf(s, "%-.*ls", slen, fp->str); break; }
@@ -240,6 +239,31 @@ static char *dp_media(char *s, struct efi_device_path *dp) return s; }
+/*
- Return a maximum size of buffer to be allocated for conversion
- @dp device path or node
- @return maximum size of buffer
- */
+static size_t efi_dp_text_max(struct efi_device_path *dp) +{
- /*
* We don't have be very accurate here.
* Currently, there are only two cases where a maximum size
* of buffer may go over MAX_NODE_LEN.
*/
- if (dp->type == DEVICE_PATH_TYPE_HARDWARE_DEVICE &&
dp->sub_type == DEVICE_PATH_SUB_TYPE_VENDOR)
/* VenHw(<guid>, xxxxxxxx...) */
return 8 + 36 + (dp->length - sizeof(*dp)) * 2 + 1;
Isn't this off by factor 2?
Do you mean "(8 + 36 + (dp->length - sizeof(*dp)) * 2 + 1) * 2"? If so, no because a buffer allocated here will eventually hold a u8 string. (See dp_xxx().) Then, efi_str_to_u16() will convert it to u16 string.
Meanwhile, + return dp->length - sizeof(*dp) + 2 /* u16 NULL */; should be return dp->length - sizeof(*dp) + 1/; or more optimistically, return (dp->length - sizeof(*dp)) / 2 + 1/;
Thanks, -Takahiro Akashi
L"VenHw(,)" is 16 bytes long. L"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxxxxxx" is 72 bytes long Each L"xx" is 4 bytes long. The terminating L'\0' is 2 bytes long. The length of u16[] cannot be odd.
Best regards
Heinrich
- if (dp->type == DEVICE_PATH_TYPE_MEDIA_DEVICE &&
dp->sub_type == DEVICE_PATH_SUB_TYPE_FILE_PATH)
return dp->length - sizeof(*dp) + 2 /* u16 NULL */;
- return MAX_NODE_LEN;
+}
/*
- Converts a single node to a char string.
@@ -293,16 +317,27 @@ static uint16_t EFIAPI *efi_convert_device_node_to_text( bool display_only, bool allow_shortcuts) {
- char str[MAX_NODE_LEN];
char *str;
size_t str_size; uint16_t *text = NULL;
EFI_ENTRY("%p, %d, %d", device_node, display_only, allow_shortcuts);
if (!device_node) goto out;
str_size = efi_dp_text_max(device_node);
if (!str_size)
goto out;
str = malloc(str_size);
if (!str)
goto out;
efi_convert_single_device_node_to_text(str, device_node);
text = efi_str_to_u16(str);
free(str);
out: EFI_EXIT(EFI_SUCCESS); @@ -327,24 +362,59 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text( bool allow_shortcuts) { uint16_t *text = NULL;
- char buffer[MAX_PATH_LEN];
- char *str = buffer;
char *buffer = NULL, *str;
size_t buf_size, buf_left;
efi_status_t ret = EFI_SUCCESS;
EFI_ENTRY("%p, %d, %d", device_path, display_only, allow_shortcuts);
- if (!device_path)
- if (!device_path) {
goto out;ret = EFI_INVALID_PARAMETER;
- while (device_path &&
str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
}
buf_size = 1024;
buf_left = buf_size;
buffer = malloc(buf_size);
if (!buffer) {
ret = EFI_OUT_OF_RESOURCES;
goto out;
}
str = buffer;
while (device_path) {
char *buf_new;
size_t buf_new_size, dp_text_len;
*str++ = '/';
buf_left--;
dp_text_len = efi_dp_text_max(device_path);
if (buf_left < dp_text_len) {
buf_new_size = buf_size + dp_text_len;
buf_new = malloc(buf_new_size);
if (!buf_new) {
ret = EFI_OUT_OF_RESOURCES;
goto out;
}
memcpy(buf_new, buffer, str - buffer);
buf_left = buf_new_size - (str - buffer);
str = buf_new + (str - buffer);
free(buffer);
buffer = buf_new;
}
str = efi_convert_single_device_node_to_text(str, device_path);
device_path = efi_dp_next(device_path); }
text = efi_str_to_u16(buffer);
out:
- EFI_EXIT(EFI_SUCCESS);
- free(buffer);
- EFI_EXIT(ret);
- return text;
}

On 10/10/19 2:50 AM, AKASHI Takahiro wrote:
On Wed, Oct 09, 2019 at 07:41:40PM +0200, Heinrich Schuchardt wrote:
On 10/9/19 9:19 AM, AKASHI Takahiro wrote:
There is no practical reason to set a maxmum length of text either for file path or whole device path in device path-to-text conversion.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_device_path_to_text.c | 90 +++++++++++++++++++++--- 1 file changed, 80 insertions(+), 10 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index b158641e3043..75aabe66bf77 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -7,12 +7,12 @@
#include <common.h> #include <efi_loader.h> +#include <malloc.h>
#define MAC_OUTPUT_LEN 22 #define UNKNOWN_OUTPUT_LEN 23
#define MAX_NODE_LEN 512 -#define MAX_PATH_LEN 1024
const efi_guid_t efi_guid_device_path_to_text_protocol = EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID; @@ -228,8 +228,7 @@ static char *dp_media(char *s, struct efi_device_path *dp) struct efi_device_path_file_path *fp = (struct efi_device_path_file_path *)dp; int slen = (dp->length - sizeof(*dp)) / 2;
if (slen > MAX_NODE_LEN - 2)
slen = MAX_NODE_LEN - 2;
- s += sprintf(s, "%-.*ls", slen, fp->str); break; }
@@ -240,6 +239,31 @@ static char *dp_media(char *s, struct efi_device_path *dp) return s; }
+/*
- Return a maximum size of buffer to be allocated for conversion
- @dp device path or node
- @return maximum size of buffer
- */
+static size_t efi_dp_text_max(struct efi_device_path *dp) +{
- /*
* We don't have be very accurate here.
* Currently, there are only two cases where a maximum size
* of buffer may go over MAX_NODE_LEN.
*/
- if (dp->type == DEVICE_PATH_TYPE_HARDWARE_DEVICE &&
dp->sub_type == DEVICE_PATH_SUB_TYPE_VENDOR)
/* VenHw(<guid>, xxxxxxxx...) */
return 8 + 36 + (dp->length - sizeof(*dp)) * 2 + 1;
Isn't this off by factor 2?
Do you mean "(8 + 36 + (dp->length - sizeof(*dp)) * 2 + 1) * 2"? If so, no because a buffer allocated here will eventually hold a u8 string. (See dp_xxx().) Then, efi_str_to_u16() will convert it to u16 string.
If you are counting UTF-8 bytes here, please, mention it in the function description.
In this case the next statement is wrong. The terminating '\0' would be only one byte while a single UTF-16 word may result in up to 3 UTF-8 bytes,e.g:
た in UTF-8: E3 81 9F, in UTF-16: 0x305F.
In dp_media() in case of DEVICE_PATH_SUB_TYPE_FILE_PATH we use
int slen = (dp->length - sizeof(*dp)) / 2;
which of cause is also incorrect.
Best regards
Heinrich
Meanwhile,
return dp->length - sizeof(*dp) + 2 /* u16 NULL */;
should be return dp->length - sizeof(*dp) + 1/; or more optimistically, return (dp->length - sizeof(*dp)) / 2 + 1/;
Thanks, -Takahiro Akashi
L"VenHw(,)" is 16 bytes long. L"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxxxxxx" is 72 bytes long Each L"xx" is 4 bytes long. The terminating L'\0' is 2 bytes long. The length of u16[] cannot be odd.
Best regards
Heinrich
- if (dp->type == DEVICE_PATH_TYPE_MEDIA_DEVICE &&
dp->sub_type == DEVICE_PATH_SUB_TYPE_FILE_PATH)
return dp->length - sizeof(*dp) + 2 /* u16 NULL */;
- return MAX_NODE_LEN;
+}
- /*
- Converts a single node to a char string.
@@ -293,16 +317,27 @@ static uint16_t EFIAPI *efi_convert_device_node_to_text( bool display_only, bool allow_shortcuts) {
- char str[MAX_NODE_LEN];
char *str;
size_t str_size; uint16_t *text = NULL;
EFI_ENTRY("%p, %d, %d", device_node, display_only, allow_shortcuts);
if (!device_node) goto out;
str_size = efi_dp_text_max(device_node);
if (!str_size)
goto out;
str = malloc(str_size);
if (!str)
goto out;
efi_convert_single_device_node_to_text(str, device_node);
text = efi_str_to_u16(str);
free(str);
out: EFI_EXIT(EFI_SUCCESS);
@@ -327,24 +362,59 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text( bool allow_shortcuts) { uint16_t *text = NULL;
- char buffer[MAX_PATH_LEN];
- char *str = buffer;
char *buffer = NULL, *str;
size_t buf_size, buf_left;
efi_status_t ret = EFI_SUCCESS;
EFI_ENTRY("%p, %d, %d", device_path, display_only, allow_shortcuts);
- if (!device_path)
- if (!device_path) {
goto out;ret = EFI_INVALID_PARAMETER;
- while (device_path &&
str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
}
buf_size = 1024;
buf_left = buf_size;
buffer = malloc(buf_size);
if (!buffer) {
ret = EFI_OUT_OF_RESOURCES;
goto out;
}
str = buffer;
while (device_path) {
char *buf_new;
size_t buf_new_size, dp_text_len;
*str++ = '/';
buf_left--;
dp_text_len = efi_dp_text_max(device_path);
if (buf_left < dp_text_len) {
buf_new_size = buf_size + dp_text_len;
buf_new = malloc(buf_new_size);
if (!buf_new) {
ret = EFI_OUT_OF_RESOURCES;
goto out;
}
memcpy(buf_new, buffer, str - buffer);
buf_left = buf_new_size - (str - buffer);
str = buf_new + (str - buffer);
free(buffer);
buffer = buf_new;
}
str = efi_convert_single_device_node_to_text(str, device_path);
device_path = efi_dp_next(device_path); }
text = efi_str_to_u16(buffer);
out:
- EFI_EXIT(EFI_SUCCESS);
- free(buffer);
- EFI_EXIT(ret);
- return text; }

This patch will lift the upper limit of maximum path length.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- lib/efi_loader/efi_device_path.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 9f772fc924fb..8be7af2b1b7d 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -1021,8 +1021,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 *file_path, *s;
if (path && !file) return EFI_INVALID_PARAMETER; @@ -1046,13 +1045,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); + file_path = strdup(path); + if (!file_path) + return EFI_OUT_OF_RESOURCES; /* DOS style file path: */ - s = filename; + s = file_path; while ((s = strchr(s, '/'))) *s++ = '\'; *file = efi_dp_from_file(((!is_net && device) ? desc : NULL), - part, filename); + part, file_path); + free(file_path);
if (!file) return EFI_INVALID_PARAMETER;

On 10/9/19 9:19 AM, AKASHI Takahiro wrote:
This patch will lift the upper limit of maximum path length.
Signed-off-by: AKASHI Takahirotakahiro.akashi@linaro.org
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
participants (2)
-
AKASHI Takahiro
-
Heinrich Schuchardt