[PATCH] dm: 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.
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
---
disk/part.c | 6 +++++- test/dm/part.c | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-)
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..aff7cfe55f 100644 --- a/test/dm/part.c +++ b/test/dm/part.c @@ -46,8 +46,10 @@ static int dm_test_part(struct unit_test_state *uts)
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", "");
Re: [PATCH] dm: Add check for NULL dev_part_str
This should probably be "part: ..."
On 5/15/21 12:36 PM, 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.
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
disk/part.c | 6 +++++- test/dm/part.c | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-)
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..aff7cfe55f 100644 --- a/test/dm/part.c +++ b/test/dm/part.c @@ -46,8 +46,10 @@ static int dm_test_part(struct unit_test_state *uts)
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", "");
On 5/15/21 6:36 PM, 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.
Fixes: 7194527b6a ("cmd: fs: Use part_get_info_by_dev_and_name_or_num to parse partitions") Reported-by: Heinrich Schuchardtxypron.glpk@gmx.de Signed-off-by: Sean Andersonseanga2@gmail.com
With this patch the reported error cannot be reproduced:
=> host bind 0 ../sandbox.img => setenv bootdevice 0:1 => ls host 148 boot.scr ...
But the unit test fails after issuing:
setenv bootdevice 0:1
=> ut dm dm_test_part Test: dm_test_part: part.c test/dm/part.c:47, dm_test_part(): -19 == part_get_info_by_dev_and_name_or_num("mmc", "", &mmc_dev_desc, &part_info, 1): Expected 0xffffffed (-19), got 0xffffffa3 (-93) Test: dm_test_part: part.c (flat tree) test/dm/part.c:47, dm_test_part(): -19 == part_get_info_by_dev_and_name_or_num("mmc", "", &mmc_dev_desc, &part_info, 1): Expected 0xffffffed (-19), got 0xffffffa3 (-93) Failures: 2 =>
We get 'No such device' instead of 'Protocol not supported'.
As the environment variable bootdevice influences the test result it should be unset when setting up the test and set to its original value when tearing down the test.
Best regards
Heinrich
On 5/15/21 1:55 PM, Heinrich Schuchardt wrote:
On 5/15/21 6:36 PM, 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.
Fixes: 7194527b6a ("cmd: fs: Use part_get_info_by_dev_and_name_or_num to parse partitions") Reported-by: Heinrich Schuchardtxypron.glpk@gmx.de Signed-off-by: Sean Andersonseanga2@gmail.com
With this patch the reported error cannot be reproduced:
=> host bind 0 ../sandbox.img => setenv bootdevice 0:1 => ls host 148 boot.scr ...
But the unit test fails after issuing:
setenv bootdevice 0:1
=> ut dm dm_test_part Test: dm_test_part: part.c test/dm/part.c:47, dm_test_part(): -19 == part_get_info_by_dev_and_name_or_num("mmc", "", &mmc_dev_desc, &part_info, 1): Expected 0xffffffed (-19), got 0xffffffa3 (-93) Test: dm_test_part: part.c (flat tree) test/dm/part.c:47, dm_test_part(): -19 == part_get_info_by_dev_and_name_or_num("mmc", "", &mmc_dev_desc, &part_info, 1): Expected 0xffffffed (-19), got 0xffffffa3 (-93) Failures: 2 =>
We get 'No such device' instead of 'Protocol not supported'.
As the environment variable bootdevice influences the test result it should be unset when setting up the test and set to its original value when tearing down the test.
ugh...
This really should be handled by the unit test framework (e.g. do the equivalent of "env save; env default -a; ut; env load" but to some temporary location). Will send a v2 with a fix...
--Sean
Best regards
Heinrich
participants (2)
-
Heinrich Schuchardt -
Sean Anderson