On 8/18/26 09:33, Patrice CHOTARD wrote:
>
>
> On 8/10/26 10:08, ngotra2710 wrote:
>> 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.
>
> Hi Ngo
>
> Feel free to submit another fix regarding the issue found in stm32mp1_tests.c
>
>>
>> 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
>
> Reviewed-by: Patrice Chotard <[email protected]>
>
> Thanks
> Patrice
Applied to u-boot-stm32/main
Thanks
Patrice
