part: Add check for NULL dev_part_str

Some callers (e.g. cmd/fs.c) of fs_set_blk_dev may use a NULL dev_part_str. While blk_get_device_part_str handles this fine, part_get_info_by_dev_and_name does not. This fixes commands crashing when implicitly using bootdevice. The unit test has also been updated to set bootdevice to a known value and to restore it after we are done. Fixes: 7194527 ("cmd: fs: Use part_get_info_by_dev_and_name_or_num to parse partitions") Reported-by: Heinrich Schuchardt <[email protected]> Signed-off-by: Sean Anderson <[email protected]> Reviewed-by: Simon Glass <[email protected]>
PeterCxy · May 26, 2021 · 26de429 · 26de429
1 parent 1e78790
commit 26de429
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 8 deletions.
diff --git a/disk/part.c b/disk/part.c
@@ -714,7 +714,11 @@ static int part_get_info_by_dev_and_name(const char *dev_iface,
 	int ret;
 
 	/* Separate device and partition name specification */
-	part_str = strchr(dev_part_str, '#');
+	if (dev_part_str)
+		part_str = strchr(dev_part_str, '#');
+	else
+		part_str = NULL;
+
 	if (part_str) {
 		dup_str = strdup(dev_part_str);
 		dup_str[part_str - dev_part_str] = 0;

diff --git a/test/dm/part.c b/test/dm/part.c
@@ -11,11 +11,25 @@
 #include <dm/test.h>
 #include <test/ut.h>
 
+static inline int do_test(struct unit_test_state *uts, int expected,
+			  const char *part_str, bool whole)
+{
+	struct blk_desc *mmc_dev_desc;
+	struct disk_partition part_info;
+
+	ut_asserteq(expected,
+		    part_get_info_by_dev_and_name_or_num("mmc", part_str,
+							 &mmc_dev_desc,
+							 &part_info, whole));
+	return 0;
+}
+
 static int dm_test_part(struct unit_test_state *uts)
 {
+	char *oldbootdevice;
 	char str_disk_guid[UUID_STR_LEN + 1];
+	int ret;
 	struct blk_desc *mmc_dev_desc;
-	struct disk_partition part_info;
 	struct disk_partition parts[2] = {
 		{
 			.start = 48, /* GPT data takes up the first 34 blocks or so */
@@ -38,16 +52,22 @@ static int dm_test_part(struct unit_test_state *uts)
 	ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, parts,
 				ARRAY_SIZE(parts)));
 
-#define test(expected, part_str, whole) \
-	ut_asserteq(expected, \
-		    part_get_info_by_dev_and_name_or_num("mmc", part_str, \
-							 &mmc_dev_desc, \
-							 &part_info, whole))
+	oldbootdevice = env_get("bootdevice");
 
+#define test(expected, part_str, whole) do { \
+	ret = do_test(uts, expected, part_str, whole); \
+	if (ret) \
+		goto out; \
+} while (0)
+
+	env_set("bootdevice", NULL);
+	test(-ENODEV, NULL, true);
 	test(-ENODEV, "", true);
 	env_set("bootdevice", "0");
+	test(0, NULL, true);
 	test(0, "", true);
 	env_set("bootdevice", "1");
+	test(1, NULL, false);
 	test(1, "", false);
 	test(1, "-", false);
 	env_set("bootdevice", "");
@@ -70,7 +90,10 @@ static int dm_test_part(struct unit_test_state *uts)
 	test(-EINVAL, "1#bogus", false);
 	test(1, "1#test1", false);
 	test(2, "1#test2", false);
+	ret = 0;
 
-	return 0;
+out:
+	env_set("bootdevice", oldbootdevice);
+	return ret;
 }
 DM_TEST(dm_test_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);