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

Reply via email to