Re: [U-Boot] [PATCH v2 2/3] efi_loader: device_path: lift the upper limit in dp-to-text conversion

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) {

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