[PATCH v2] part: Add check for NULL dev_part_str

[PATCH v2] part: Add check for NULL dev_part_str

[Sean Anderson]

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: 7194527b6a ("cmd: fs: Use part_get_info_by_dev_and_name_or_num to parse partitions")
Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

Changes in v2:
- Fix unit test failing if bootdevice was set

 disk/part.c    |  6 +++++-
 test/dm/part.c | 39 +++++++++++++++++++++++++++++++--------
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/disk/part.c b/disk/part.c
index 5e7e59cf25..086da84b7f 100644
--- 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
index 051e9010b6..78dd8472c2 100644
--- a/test/dm/part.c
+++ b/test/dm/part.c
@@ -11,11 +11,25 @@
 #include <dm/test.h>
 #include <test/ut.h>
 
-static int dm_test_part(struct unit_test_state *uts)
+static inline int do_test(struct unit_test_state *uts, int expected,
+			  const char *part_str, bool whole)
 {
-	char str_disk_guid[UUID_STR_LEN + 1];
 	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 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);
-- 
2.31.0

[PATCH v2] part: Add check for NULL dev_part_str

[Simon Glass]

On Sat, 15 May 2021 at 12:14, Sean Anderson <seanga2@gmail.com> wrote:
>
> 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: 7194527b6a ("cmd: fs: Use part_get_info_by_dev_and_name_or_num to parse partitions")
> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
> Changes in v2:
> - Fix unit test failing if bootdevice was set
>
>  disk/part.c    |  6 +++++-
>  test/dm/part.c | 39 +++++++++++++++++++++++++++++++--------
>  2 files changed, 36 insertions(+), 9 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2] part: Add check for NULL dev_part_str

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

On Sat, May 15, 2021 at 02:13:54PM -0400, Sean Anderson wrote:

> 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: 7194527b6a ("cmd: fs: Use part_get_info_by_dev_and_name_or_num to parse partitions")
> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]