[PATCH 1/4] cmd: clk: test the number of argument in setfreq command

Test the number of argument in setfreq command to avoid a crash when the command setfreq is called without argument:
STM32MP> clk setfreq data abort pc : [<ddba3f18>] lr : [<ddba3f89>] reloc pc : [<c018ff18>] lr : [<c018ff89>] sp : dbaf45b8 ip : ddb1d859 fp : 00000002 r10: dbb3fd80 r9 : dbb11e90 r8 : ddbf38cc r7 : ddb39725 r6 : 00000000 r5 : 00000000 r4 : dbb3fd84 r3 : dbb3fd84 r2 : 0000000a r1 : dbaf45bc r0 : 00000011 Flags: nzCv IRQs off FIQs off Mode SVC_32 (T) Code: 4dd3 1062 85a3 ddbd (7803) 2b30 Resetting CPU ...
Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline") Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com ---
cmd/clk.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/cmd/clk.c b/cmd/clk.c index dbbdc31b35..52237791cf 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -120,6 +120,9 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc, s32 freq; struct udevice *dev;
+ if (argc != 3) + return CMD_RET_USAGE; + freq = dectoul(argv[2], NULL);
dev = clk_lookup(argv[1]);

The function clk_lookup can be replaced by a direct call to uclass_get_device_by_name for UCLASS_CLK.
This patch removes duplicated codes by the generic DM API and avoids issue in clk_lookup because result of uclass_get_device wasn't tested; when ret < 0, dev = NULL and dev->name is invalid, the next function call strcmp(name, dev->name) causes a crash.
Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com ---
cmd/clk.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/cmd/clk.c b/cmd/clk.c index 52237791cf..d615f14a84 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -99,20 +99,6 @@ static int do_clk_dump(struct cmd_tbl *cmdtp, int flag, int argc, }
#if CONFIG_IS_ENABLED(DM) && CONFIG_IS_ENABLED(CLK) -struct udevice *clk_lookup(const char *name) -{ - int i = 0; - struct udevice *dev; - - do { - uclass_get_device(UCLASS_CLK, i++, &dev); - if (!strcmp(name, dev->name)) - return dev; - } while (dev); - - return NULL; -} - static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { @@ -125,9 +111,7 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
freq = dectoul(argv[2], NULL);
- dev = clk_lookup(argv[1]); - - if (dev) + if (!uclass_get_device_by_name(UCLASS_CLK, argv[1], &dev)) clk = dev_get_clk_ptr(dev);
if (!clk) {

On 1/31/22 11:21 AM, Patrick Delaunay wrote:
The function clk_lookup can be replaced by a direct call to uclass_get_device_by_name for UCLASS_CLK.
This patch removes duplicated codes by the generic DM API and avoids issue in clk_lookup because result of uclass_get_device wasn't tested; when ret < 0, dev = NULL and dev->name is invalid, the next function call strcmp(name, dev->name) causes a crash.
Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com
cmd/clk.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/cmd/clk.c b/cmd/clk.c index 52237791cf..d615f14a84 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -99,20 +99,6 @@ static int do_clk_dump(struct cmd_tbl *cmdtp, int flag, int argc, }
#if CONFIG_IS_ENABLED(DM) && CONFIG_IS_ENABLED(CLK) -struct udevice *clk_lookup(const char *name) -{
- int i = 0;
- struct udevice *dev;
- do {
uclass_get_device(UCLASS_CLK, i++, &dev);
if (!strcmp(name, dev->name))
return dev;
- } while (dev);
- return NULL;
-}
- static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) {
@@ -125,9 +111,7 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
freq = dectoul(argv[2], NULL);
- dev = clk_lookup(argv[1]);
- if (dev)
if (!uclass_get_device_by_name(UCLASS_CLK, argv[1], &dev)) clk = dev_get_clk_ptr(dev);
if (!clk) {
Reviewed-by: Sean Anderson
(This is such a strange command, since it can only handle CCF clocks)

Update the result of do_clk_setfreq and always returns a CMD_RET_ value (-EINVAL was a possible result).
This patch avoid the CLI output "exit not allowed from main input shell."
Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline") Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com ---
cmd/clk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/clk.c b/cmd/clk.c index d615f14a84..73dea5e746 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -116,7 +116,7 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
if (!clk) { printf("clock '%s' not found.\n", argv[1]); - return -EINVAL; + return CMD_RET_FAILURE; }
freq = clk_set_rate(clk, freq);

On 1/31/22 11:21 AM, Patrick Delaunay wrote:
Update the result of do_clk_setfreq and always returns a CMD_RET_ value (-EINVAL was a possible result).
This patch avoid the CLI output "exit not allowed from main input shell."
Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline") Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com
cmd/clk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/clk.c b/cmd/clk.c index d615f14a84..73dea5e746 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -116,7 +116,7 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
if (!clk) { printf("clock '%s' not found.\n", argv[1]);
return -EINVAL;
return CMD_RET_FAILURE;
}
freq = clk_set_rate(clk, freq);
Reviewed-by: Sean Anderson seanga2@gmail.com

Fix the long help message for "clk setfreq" command
Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline") Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com ---
cmd/clk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/clk.c b/cmd/clk.c index 73dea5e746..a483fd8981 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -160,7 +160,7 @@ static int do_clk(struct cmd_tbl *cmdtp, int flag, int argc, #ifdef CONFIG_SYS_LONGHELP static char clk_help_text[] = "dump - Print clock frequencies\n" - "setfreq [clk] [freq] - Set clock frequency"; + "clk setfreq [clk] [freq] - Set clock frequency"; #endif
U_BOOT_CMD(clk, 4, 1, do_clk, "CLK sub-system", clk_help_text);

On 1/31/22 11:21 AM, Patrick Delaunay wrote:
Fix the long help message for "clk setfreq" command
Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline") Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com
cmd/clk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/clk.c b/cmd/clk.c index 73dea5e746..a483fd8981 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -160,7 +160,7 @@ static int do_clk(struct cmd_tbl *cmdtp, int flag, int argc, #ifdef CONFIG_SYS_LONGHELP static char clk_help_text[] = "dump - Print clock frequencies\n"
- "setfreq [clk] [freq] - Set clock frequency";
"clk setfreq [clk] [freq] - Set clock frequency"; #endif
U_BOOT_CMD(clk, 4, 1, do_clk, "CLK sub-system", clk_help_text);
Reviewed-by: Sean Anderson seanga2@gmail.com

On 1/31/22 11:21 AM, Patrick Delaunay wrote:
Test the number of argument in setfreq command to avoid a crash when the command setfreq is called without argument:
STM32MP> clk setfreq data abort pc : [<ddba3f18>] lr : [<ddba3f89>] reloc pc : [<c018ff18>] lr : [<c018ff89>] sp : dbaf45b8 ip : ddb1d859 fp : 00000002 r10: dbb3fd80 r9 : dbb11e90 r8 : ddbf38cc r7 : ddb39725 r6 : 00000000 r5 : 00000000 r4 : dbb3fd84 r3 : dbb3fd84 r2 : 0000000a r1 : dbaf45bc r0 : 00000011 Flags: nzCv IRQs off FIQs off Mode SVC_32 (T) Code: 4dd3 1062 85a3 ddbd (7803) 2b30 Resetting CPU ...
Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline") Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com
cmd/clk.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/cmd/clk.c b/cmd/clk.c index dbbdc31b35..52237791cf 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -120,6 +120,9 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc, s32 freq; struct udevice *dev;
if (argc != 3)
return CMD_RET_USAGE;
freq = dectoul(argv[2], NULL);
dev = clk_lookup(argv[1]);
Reviewed-by: Sean Anderson seanga2@gmail.com

On Mon, 31 Jan 2022 17:21:37 +0100, Patrick Delaunay wrote:
Test the number of argument in setfreq command to avoid a crash when the command setfreq is called without argument:
STM32MP> clk setfreq data abort pc : [<ddba3f18>] lr : [<ddba3f89>] reloc pc : [<c018ff18>] lr : [<c018ff89>] sp : dbaf45b8 ip : ddb1d859 fp : 00000002 r10: dbb3fd80 r9 : dbb11e90 r8 : ddbf38cc r7 : ddb39725 r6 : 00000000 r5 : 00000000 r4 : dbb3fd84 r3 : dbb3fd84 r2 : 0000000a r1 : dbaf45bc r0 : 00000011 Flags: nzCv IRQs off FIQs off Mode SVC_32 (T) Code: 4dd3 1062 85a3 ddbd (7803) 2b30 Resetting CPU ...
[...]
Applied, thanks!
[1/4] cmd: clk: test the number of argument in setfreq command commit: 3386fb1e485da7f206488ed206a61ae811885d90 [2/4] cmd: clk: replace clk_lookup by uclass_get_device_by_name commit: afcc26140bc6bff7c23ce02dbba7882c97d2c14a [3/4] cmd: clk: update result of do_clk_setfreq commit: 534859ac6b2fecb631457736e77e9d0df1e57616 [4/4] cmd: clk: fix long help message commit: 92a1bba85761e4dd5c0647c48048423bceef4749
Best regards,
participants (2)
-
Patrick Delaunay
-
Sean Anderson