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

[Lothar Waßmann]

Hi,

On Sat,  1 Jul 2017 15:44:44 -0700 alison at peloton-tech.com wrote:
> From: Alison Chaiken <alison at 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 at entry=0x7fffffffdbe8, partitions=partitions at entry=0x7fffffffdbd8,
>     parts_count=parts_count at 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 at entry=0x726c04 <flag_repeat>, ticks=ticks at entry=0x0) at common/command.c:539
> 
> Suggested-by: Lothar Waßmann <LW at karo-electronics.de>
> Signed-off-by: Alison Chaiken <alison at 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