[PATCH v2] common: cli_hush: fix console_buffer overflow on boot retry
Sverdlin, Alexander
alexander.sverdlin at siemens.com
Tue Mar 31 08:29:21 CEST 2026
Hi Ngo,
thanks for the patch!
On Tue, 2026-03-31 at 10:25 +0700, Ngo Luong Thanh Tra wrote:
> Add const_strcpy() macro to <linux/build_bug.h> that enforces at
> compile time that the destination is a char array (not a pointer),
> the source is a string literal, and the source fits in the
> destination including the NUL terminator. It uses __builtin_strcpy()
> so the compiler can optimize the copy.
>
> Fix the console_buffer extern declaration in <console.h> to include
> the array size so that sizeof(console_buffer) is valid at call sites.
>
> Replace the unbounded strcpy() in cli_hush.c with const_strcpy() to
> catch at compile time any configuration where CONFIG_SYS_CBSIZE is
> smaller than the boot retry command string.
>
> Fixes: 657e19f8f2dd ("cli_hush: support running bootcmd on boot retry")
> Signed-off-by: Ngo Luong Thanh Tra <S4210155 at student.rmit.edu.au>
> To: u-boot at lists.denx.de
The patch itself looks good to me and you can add my
Reviewed-by: Alexander Sverdlin <alexander.sverdlin at siemens.com>
There is a small issue with headers and tags, maybe
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
could help here, please refer to optional "From:" tag, which would allow
you to avoid the following checkpatch.pl warning maintainers would get:
WARNING: From:/Signed-off-by: email address mismatch: 'From: Ngo Luong Thanh Tra <ngotra27101996 at gmail.com>' != 'Signed-off-by: Ngo Luong Thanh Tra <S4210155 at student.rmit.edu.au>'
> ---
>
> Changes in v2:
> - Replace strlcpy() + BUILD_BUG_ON approach with a new const_strcpy()
> macro that enforces compile-time safety for string literal copies
> - Fix console_buffer extern declaration in <console.h> to include
> array size so sizeof(console_buffer) is valid at call sites
> - Address review from Rasmus Villemoes
>
> common/cli_hush.c | 3 ++-
> include/console.h | 3 ++-
> include/linux/build_bug.h | 23 +++++++++++++++++++++++
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/common/cli_hush.c b/common/cli_hush.c
> index 7bd6943d3ed..048577cb40a 100644
> --- a/common/cli_hush.c
> +++ b/common/cli_hush.c
> @@ -84,6 +84,7 @@
> #include <cli_hush.h>
> #include <command.h> /* find_cmd */
> #include <asm/global_data.h>
> +#include <linux/build_bug.h>
> #endif
> #ifndef __U_BOOT__
> #include <ctype.h> /* isalpha, isdigit */
> @@ -1029,7 +1030,7 @@ static void get_user_input(struct in_str *i)
> # ifdef CONFIG_RESET_TO_RETRY
> do_reset(NULL, 0, 0, NULL);
> # elif IS_ENABLED(CONFIG_RETRY_BOOTCMD)
> - strcpy(console_buffer, "run bootcmd\n");
> + const_strcpy(console_buffer, "run bootcmd\n");
> # else
> # error "This only works with CONFIG_RESET_TO_RETRY or CONFIG_BOOT_RETRY_COMMAND enabled"
> # endif
> diff --git a/include/console.h b/include/console.h
> index 8d0d7bb8a4c..97ccf5e5f6a 100644
> --- a/include/console.h
> +++ b/include/console.h
> @@ -10,8 +10,9 @@
> #include <stdbool.h>
> #include <stdio_dev.h>
> #include <linux/errno.h>
> +#include <config.h>
>
> -extern char console_buffer[];
> +extern char console_buffer[CONFIG_SYS_CBSIZE + 1];
>
> /* common/console.c */
> int console_init_f(void); /* Before relocation; uses the serial stuff */
> diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
> index 20c2dc7f4bd..d775bf1bf91 100644
> --- a/include/linux/build_bug.h
> +++ b/include/linux/build_bug.h
> @@ -76,4 +76,27 @@
> #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
> #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>
> +/**
> + * const_strcpy - Copy a string literal to a char array with compile-time checks
> + * @d: destination char array (must be a char array, not a pointer)
> + * @s: source string literal
> + *
> + * Enforces at compile time that:
> + * (a) @d is a char array, not a pointer
> + * (b) @s is a string literal (adjacent string concatenation trick)
> + * (c) @s fits in @d including the NUL terminator
> + *
> + * Uses __builtin_strcpy() so the compiler can optimize the copy into
> + * immediate stores rather than emitting a function call.
> + *
> + * Note: @s is used twice in the macro expansion but this is intentional
> + * and safe: the ("" s "") trick enforces at compile time that @s is a
> + * string literal, and string literals have no side effects.
> + */
> +#define const_strcpy(d, s) ({ \
> + BUILD_BUG_ON(__builtin_types_compatible_p(typeof(d), char *)); \
> + BUILD_BUG_ON(sizeof(d) < sizeof("" s "")); \
> + __builtin_strcpy(d, s); \
> +})
> +
> #endif /* _LINUX_BUILD_BUG_H */
--
Alexander Sverdlin
Siemens AG
www.siemens.com
More information about the U-Boot
mailing list