From: Ngo Luong Thanh Tra <[email protected]>
stm32mp1_ddr_subcmd() formats a rejected argument into a 50-byte stack
buffer with an unbounded sprintf():
char string[50] = "";
...
sprintf(string, "invalid argument %s", argv[1]);
argv[1] comes from cli_simple_parse_line() over a console line held in
buffer[CONFIG_SYS_CBSIZE], which is commonly 256 bytes or more. The
fixed prefix takes 17 bytes, leaving 32 for the argument and its NUL, so
any invalid argument longer than 32 characters writes past the end of
string and corrupts the stack of the DDR interactive console.
Use snprintf() with sizeof(string). The sibling call a few lines below
formats only integers and cannot overflow, so it is left alone.
Fixes: 0d447524425e ("stm32mp1: ram: add tests in DDR interactive mode")
Signed-off-by: Ngo Luong Thanh Tra <[email protected]>
Cc: Patrick Delaunay <[email protected]>
Cc: Patrice Chotard <[email protected]>
Cc: Tom Rini <[email protected]>
---
Note, not for the commit message: the same 50-byte buffer is handed to the
test functions in drivers/ram/stm32mp1/stm32mp1_tests.c, which write into it
with further unbounded sprintf() calls, several of them formatting %s from
argv (lines 28, 33, 38, 60, 81, 86, 90 and 113). Those cannot be fixed the
same way because the buffer arrives there as a char * parameter, so sizeof()
is not available at the call site. Fixing them properly needs either a size
parameter alongside the pointer or a shared constant for the buffer length,
which seemed like a separate change rather than something to fold in here.
Happy to follow up with that if you would like it done in one go.
drivers/ram/stm32mp1/stm32mp1_interactive.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ram/stm32mp1/stm32mp1_interactive.c
b/drivers/ram/stm32mp1/stm32mp1_interactive.c
index 6340afbb87..3154fde6bf 100644
--- a/drivers/ram/stm32mp1/stm32mp1_interactive.c
+++ b/drivers/ram/stm32mp1/stm32mp1_interactive.c
@@ -334,8 +334,8 @@ static void stm32mp1_ddr_subcmd(struct ddr_info *priv,
if ((strict_strtoul(argv[1], 0, &value) < 0) ||
value >= array_nb) {
- sprintf(string, "invalid argument %s",
- argv[1]);
+ snprintf(string, sizeof(string), "invalid argument %s",
+ argv[1]);
result = TEST_FAILED;
goto end;
}
base-commit: ece349ade2973e220f524ce59e59711cc919263f
prerequisite-patch-id: 53e6ea149bf19676d9f2becfa13157b07bee89fc
--
2.53.0
