diff options
author | Alison Chaiken <alison@peloton-tech.com> | 2017-07-04 11:19:46 -0700 |
---|---|---|
committer | Tom Rini <trini@konsulko.com> | 2017-08-04 20:38:32 -0400 |
commit | 2fcaa413b3f63f6671c90896df9a1bbd84390a4e (patch) | |
tree | 8664aa779de0d46ef436c90ca75e0400e91fbbe3 /cmd/gpt.c | |
parent | 203f9b48adaf5ad5372ac72d82ab14465c82ee8d (diff) |
gpt: harden set_gpt_info() against non NULL-terminated strings
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>
Diffstat (limited to 'cmd/gpt.c')
-rw-r--r-- | cmd/gpt.c | 63 |
1 files changed, 36 insertions, 27 deletions
diff --git a/cmd/gpt.c b/cmd/gpt.c index dab47c71f96..6f08f611168 100644 --- a/cmd/gpt.c +++ b/cmd/gpt.c @@ -156,6 +156,25 @@ static bool found_key(const char *str, const char *key) return result; } +static int calc_parts_list_len(int numparts) +{ + int partlistlen = UUID_STR_LEN + 1 + strlen("uuid_disk="); + /* for the comma */ + partlistlen++; + + /* per-partition additions; numparts starts at 1, so this should be correct */ + partlistlen += numparts * (strlen("name=,") + PART_NAME_LEN + 1); + /* see part.h for definition of struct disk_partition */ + partlistlen += numparts * (strlen("start=MiB,") + sizeof(lbaint_t) + 1); + partlistlen += numparts * (strlen("size=MiB,") + sizeof(lbaint_t) + 1); + partlistlen += numparts * (strlen("uuid=;") + UUID_STR_LEN + 1); + /* for the terminating null */ + partlistlen++; + debug("Length of partitions_list is %d for %d partitions\n", partlistlen, + numparts); + return partlistlen; +} + #ifdef CONFIG_CMD_GPT_RENAME static void del_gpt_info(void) { @@ -234,25 +253,6 @@ static void print_gpt_info(void) } } -static int calc_parts_list_len(int numparts) -{ - int partlistlen = UUID_STR_LEN + 1 + strlen("uuid_disk="); - /* for the comma */ - partlistlen++; - - /* per-partition additions; numparts starts at 1, so this should be correct */ - partlistlen += numparts * (strlen("name=,") + PART_NAME_LEN + 1); - /* see part.h for definition of struct disk_partition */ - partlistlen += numparts * (strlen("start=MiB,") + sizeof(lbaint_t) + 1); - partlistlen += numparts * (strlen("size=MiB,") + sizeof(lbaint_t) + 1); - partlistlen += numparts * (strlen("uuid=;") + UUID_STR_LEN + 1); - /* for the terminating null */ - partlistlen++; - debug("Length of partitions_list is %d for %d partitions\n", partlistlen, - numparts); - return partlistlen; -} - /* * create the string that upstream 'gpt write' command will accept as an * argument @@ -385,6 +385,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); debug("%s: lba num: 0x%x %d\n", __func__, (unsigned int)dev_desc->lba, (unsigned int)dev_desc->lba); @@ -402,6 +403,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); @@ -416,10 +419,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; + } + if (strnlen(s, max_str_part) == 0) return -3; - i = strlen(s) - 1; + i = strnlen(s, max_str_part) - 1; if (s[i] == ';') s[i] = '\0'; @@ -433,6 +440,8 @@ static int set_gpt_info(struct blk_desc *dev_desc, /* allocate memory for partitions */ parts = calloc(sizeof(disk_partition_t), p_count); + if (parts == NULL) + return -ENOMEM; /* retrieve partitions data from string */ for (i = 0; i < p_count; i++) { @@ -454,12 +463,12 @@ static int set_gpt_info(struct blk_desc *dev_desc, } else { if (extract_env(val, &p)) p = val; - if (strlen(p) >= sizeof(parts[i].uuid)) { + if (strnlen(p, max_str_part) >= sizeof(parts[i].uuid)) { printf("Wrong uuid format for partition %d\n", i); errno = -4; goto err; } - strcpy((char *)parts[i].uuid, p); + strncpy((char *)parts[i].uuid, p, max_str_part); free(val); } #ifdef CONFIG_PARTITION_TYPE_GUID @@ -469,13 +478,13 @@ static int set_gpt_info(struct blk_desc *dev_desc, /* 'type' is optional */ if (extract_env(val, &p)) p = val; - if (strlen(p) >= sizeof(parts[i].type_guid)) { + if (strnlen(p, max_str_part) >= sizeof(parts[i].type_guid)) { printf("Wrong type guid format for partition %d\n", i); errno = -4; goto err; } - strcpy((char *)parts[i].type_guid, p); + strncpy((char *)parts[i].type_guid, p, max_str_part); free(val); } #endif @@ -487,11 +496,11 @@ static int set_gpt_info(struct blk_desc *dev_desc, } if (extract_env(val, &p)) p = val; - if (strlen(p) >= sizeof(parts[i].name)) { + if (strnlen(p, max_str_part) >= sizeof(parts[i].name)) { errno = -4; goto err; } - strcpy((char *)parts[i].name, p); + strncpy((char *)parts[i].name, p, max_str_part); free(val); /* size */ |