Re: [U-Boot] [PATCH 10/10] gpt: harden set_gpt_info() against non NULL-terminated strings

3 Jul 2017

Hi,
On Sat,  1 Jul 2017 15:44:44 -0700 alison@peloton-tech.com wrote:
...
From: Alison Chaiken alison@peloton-tech.com
Strings read from devices may sometimes fail to be
NULL-terminated.   The functions in lib/string.c are subject to
failure in this case.   Protect against observed failures in
set_gpt_info() by switching to length-checking variants with a length
limit of the maximum possible partition table length.  At the same
time, add a few checks for NULL string pointers.
Here is an example as observed in sandbox under GDB:
=> gpt verify host 0 $partitions
Program received signal SIGSEGV, Segmentation fault.
0x0000000000477747 in strlen (s=0x0) at lib/string.c:267
267             for (sc = s; *sc != '\0'; ++sc)
(gdb) bt
#0  0x0000000000477747 in strlen (s=0x0) at lib/string.c:267
#1  0x00000000004140b2 in set_gpt_info (str_part=<optimized out>,
str_disk_guid=str_disk_guid@entry=0x7fffffffdbe8, partitions=partitions@entry=0x7fffffffdbd8,
parts_count=parts_count@entry=0x7fffffffdbcf "", dev_desc=<optimized out>) at cmd/gpt.c:415
#2  0x00000000004145b9 in gpt_verify (str_part=<optimized out>, blk_dev_desc=0x7fffef09a9d0) at cmd/gpt.c:580
#3  do_gpt (cmdtp=<optimized out>, flag=<optimized out>, argc=<optimized out>, argv=0x7fffef09a8f0)
at cmd/gpt.c:783
#4  0x00000000004295b0 in cmd_call (argv=0x7fffef09a8f0, argc=0x5, flag=<optimized out>,
cmdtp=0x714e20 <_u_boot_list_2_cmd_2_gpt>) at common/command.c:500
#5  cmd_process (flag=<optimized out>, argc=0x5, argv=0x7fffef09a8f0,
repeatable=repeatable@entry=0x726c04 <flag_repeat>, ticks=ticks@entry=0x0) at common/command.c:539


Suggested-by: Lothar Waßmann LW@karo-electronics.de
Signed-off-by: Alison Chaiken alison@peloton-tech.com

cmd/gpt.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/cmd/gpt.c b/cmd/gpt.c
index 73bf273..8bd7bdf 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -233,7 +233,7 @@ static void print_gpt_info(void)
   }
 }
-#ifdef CONFIG_CMD_GPT_RENAME



static int calc_parts_list_len(int numparts)
 {
   int partlistlen = UUID_STR_LEN + 1 + strlen("uuid_disk=");
@@ -253,6 +253,7 @@ static int calc_parts_list_len(int numparts)
   return partlistlen;
 }
+#ifdef CONFIG_CMD_GPT_RENAME
 /*

create the string that upstream 'gpt write' command will accept as an
argument

@@ -381,6 +382,7 @@ static int set_gpt_info(struct blk_desc *dev_desc,
   int errno = 0;
   uint64_t size_ll, start_ll;
   lbaint_t offset = 0;

   int max_str_part = calc_parts_list_len(MAX_SEARCH_PARTITIONS);




indentation should use tabs not spaces (scripts/checkpatch.pl would tell
you).
...
debug("%s:  lba num: 0x%x %d\n", __func__,
         (unsigned int)dev_desc->lba, (unsigned int)dev_desc->lba);
@@ -398,6 +400,8 @@ static int set_gpt_info(struct blk_desc *dev_desc,
   if (!val) {
 #ifdef CONFIG_RANDOM_UUID
   	*str_disk_guid = malloc(UUID_STR_LEN + 1);

if (str_disk_guid == NULL)


	return -ENOMEM;

gen_rand_uuid_str(*str_disk_guid, UUID_STR_FORMAT_STD);

#else
   	free(str);
@@ -412,10 +416,14 @@ static int set_gpt_info(struct blk_desc *dev_desc,
   	/* Move s to first partition */
   	strsep(&s, ";");
   }

if (strlen(s) == 0)


if (s == NULL) {
           printf("Error: is the partitions string NULL-terminated?\n");


return -EINVAL;




dto.
Lothar Waßmann

    