[PATCH v2] 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: 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);

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

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!
participants (3)
-
Sean Anderson
-
Simon Glass
-
Tom Rini