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.aka...@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) { > >+ 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; > > } > > > > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot