[PATCH] ram: stm32mp1: bound the invalid-argument message in stm32mp1_ddr_subcmd()
ngotra2710 <[email protected]>
| Newsgroups | gmane.comp.boot-loaders.u-boot.general,gmane.comp.boot-loaders.u-boot |
|---|---|
| Message-ID | <[email protected]> |
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