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>

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>
Alison Chaiken · Tom Rini
1 parent 203f9b48ad
Showing 1 changed file with 36 additions and 27 deletions Side-by-side Diff
cmd/gpt.c
@@ -156,6 +156,25 @@
 	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 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 @@
 	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 @@
 	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 @@
 		/* 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 @@
  
 	/* 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 @@
 		} 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 @@
 			/* '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 @@
 		}
 		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 */
...	...	@@ -156,6 +156,25 @@
156	156	return result;
157	157	}
158	158
	159	+static int calc_parts_list_len(int numparts)
	160	+{
	161	+ int partlistlen = UUID_STR_LEN + 1 + strlen("uuid_disk=");
	162	+ /* for the comma */
	163	+ partlistlen++;
	164	+
	165	+ /* per-partition additions; numparts starts at 1, so this should be correct */
	166	+ partlistlen += numparts * (strlen("name=,") + PART_NAME_LEN + 1);
	167	+ /* see part.h for definition of struct disk_partition */
	168	+ partlistlen += numparts * (strlen("start=MiB,") + sizeof(lbaint_t) + 1);
	169	+ partlistlen += numparts * (strlen("size=MiB,") + sizeof(lbaint_t) + 1);
	170	+ partlistlen += numparts * (strlen("uuid=;") + UUID_STR_LEN + 1);
	171	+ /* for the terminating null */
	172	+ partlistlen++;
	173	+ debug("Length of partitions_list is %d for %d partitions\n", partlistlen,
	174	+ numparts);
	175	+ return partlistlen;
	176	+}
	177	+
159	178	#ifdef CONFIG_CMD_GPT_RENAME
160	179	static void del_gpt_info(void)
161	180	{
...	...	@@ -234,25 +253,6 @@
234	253	}
235	254	}
236	255
237		-static int calc_parts_list_len(int numparts)
238		-{
239		- int partlistlen = UUID_STR_LEN + 1 + strlen("uuid_disk=");
240		- /* for the comma */
241		- partlistlen++;
242		-
243		- /* per-partition additions; numparts starts at 1, so this should be correct */
244		- partlistlen += numparts * (strlen("name=,") + PART_NAME_LEN + 1);
245		- /* see part.h for definition of struct disk_partition */
246		- partlistlen += numparts * (strlen("start=MiB,") + sizeof(lbaint_t) + 1);
247		- partlistlen += numparts * (strlen("size=MiB,") + sizeof(lbaint_t) + 1);
248		- partlistlen += numparts * (strlen("uuid=;") + UUID_STR_LEN + 1);
249		- /* for the terminating null */
250		- partlistlen++;
251		- debug("Length of partitions_list is %d for %d partitions\n", partlistlen,
252		- numparts);
253		- return partlistlen;
254		-}
255		-
256	256	/*
257	257	* create the string that upstream 'gpt write' command will accept as an
258	258	* argument
...	...	@@ -385,6 +385,7 @@
385	385	int errno = 0;
386	386	uint64_t size_ll, start_ll;
387	387	lbaint_t offset = 0;
	388	+ int max_str_part = calc_parts_list_len(MAX_SEARCH_PARTITIONS);
388	389
389	390	debug("%s: lba num: 0x%x %d\n", __func__,
390	391	(unsigned int)dev_desc->lba, (unsigned int)dev_desc->lba);
...	...	@@ -402,6 +403,8 @@
402	403	if (!val) {
403	404	#ifdef CONFIG_RANDOM_UUID
404	405	*str_disk_guid = malloc(UUID_STR_LEN + 1);
	406	+ if (str_disk_guid == NULL)
	407	+ return -ENOMEM;
405	408	gen_rand_uuid_str(*str_disk_guid, UUID_STR_FORMAT_STD);
406	409	#else
407	410	free(str);
408	411
...	...	@@ -416,10 +419,14 @@
416	419	/* Move s to first partition */
417	420	strsep(&s, ";");
418	421	}
419		- if (strlen(s) == 0)
	422	+ if (s == NULL) {
	423	+ printf("Error: is the partitions string NULL-terminated?\n");
	424	+ return -EINVAL;
	425	+ }
	426	+ if (strnlen(s, max_str_part) == 0)
420	427	return -3;
421	428
422		- i = strlen(s) - 1;
	429	+ i = strnlen(s, max_str_part) - 1;
423	430	if (s[i] == ';')
424	431	s[i] = '\0';
425	432
...	...	@@ -433,6 +440,8 @@
433	440
434	441	/* allocate memory for partitions */
435	442	parts = calloc(sizeof(disk_partition_t), p_count);
	443	+ if (parts == NULL)
	444	+ return -ENOMEM;
436	445
437	446	/* retrieve partitions data from string */
438	447	for (i = 0; i < p_count; i++) {
439	448
...	...	@@ -454,12 +463,12 @@
454	463	} else {
455	464	if (extract_env(val, &p))
456	465	p = val;
457		- if (strlen(p) >= sizeof(parts[i].uuid)) {
	466	+ if (strnlen(p, max_str_part) >= sizeof(parts[i].uuid)) {
458	467	printf("Wrong uuid format for partition %d\n", i);
459	468	errno = -4;
460	469	goto err;
461	470	}
462		- strcpy((char *)parts[i].uuid, p);
	471	+ strncpy((char *)parts[i].uuid, p, max_str_part);
463	472	free(val);
464	473	}
465	474	#ifdef CONFIG_PARTITION_TYPE_GUID
466	475
...	...	@@ -469,13 +478,13 @@
469	478	/* 'type' is optional */
470	479	if (extract_env(val, &p))
471	480	p = val;
472		- if (strlen(p) >= sizeof(parts[i].type_guid)) {
	481	+ if (strnlen(p, max_str_part) >= sizeof(parts[i].type_guid)) {
473	482	printf("Wrong type guid format for partition %d\n",
474	483	i);
475	484	errno = -4;
476	485	goto err;
477	486	}
478		- strcpy((char *)parts[i].type_guid, p);
	487	+ strncpy((char *)parts[i].type_guid, p, max_str_part);
479	488	free(val);
480	489	}
481	490	#endif
482	491
...	...	@@ -487,11 +496,11 @@
487	496	}
488	497	if (extract_env(val, &p))
489	498	p = val;
490		- if (strlen(p) >= sizeof(parts[i].name)) {
	499	+ if (strnlen(p, max_str_part) >= sizeof(parts[i].name)) {
491	500	errno = -4;
492	501	goto err;
493	502	}
494		- strcpy((char *)parts[i].name, p);
	503	+ strncpy((char *)parts[i].name, p, max_str_part);
495	504	free(val);
496	505
497	506	/* size */