From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lothar =?UTF-8?B?V2HDn21hbm4=?= Date: Mon, 3 Jul 2017 08:37:11 +0200 Subject: [U-Boot] [PATCH 10/10] gpt: harden set_gpt_info() against non NULL-terminated strings In-Reply-To: <1498949084-28304-1-git-send-email-alison@peloton-tech.com> References: <20170627111236.72d6b55e@karo-electronics.de> <1498949084-28304-1-git-send-email-alison@peloton-tech.com> Message-ID: <20170703083711.02c040ce@karo-electronics.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de Hi, On Sat, 1 Jul 2017 15:44:44 -0700 alison at peloton-tech.com wrote: > From: Alison Chaiken > > 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=, > str_disk_guid=str_disk_guid at entry=0x7fffffffdbe8, partitions=partitions at entry=0x7fffffffdbd8, > parts_count=parts_count at entry=0x7fffffffdbcf "", dev_desc=) at cmd/gpt.c:415 > #2 0x00000000004145b9 in gpt_verify (str_part=, blk_dev_desc=0x7fffef09a9d0) at cmd/gpt.c:580 > #3 do_gpt (cmdtp=, flag=, argc=, argv=0x7fffef09a8f0) > at cmd/gpt.c:783 > #4 0x00000000004295b0 in cmd_call (argv=0x7fffef09a8f0, argc=0x5, flag=, > cmdtp=0x714e20 <_u_boot_list_2_cmd_2_gpt>) at common/command.c:500 > #5 cmd_process (flag=, argc=0x5, argv=0x7fffef09a8f0, > repeatable=repeatable at entry=0x726c04 , ticks=ticks at entry=0x0) at common/command.c:539 > > Suggested-by: Lothar Waßmann > Signed-off-by: Alison Chaiken > --- > 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