[PATCH v2] common: cli_hush: fix console_buffer overflow on boot retry

[PATCH v2] common: cli_hush: fix console_buffer overflow on boot retry

[Ngo Luong Thanh Tra]

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@student.rmit.edu.au>
To: u-boot@lists.denx.de
---

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 */
-- 
2.53.0

base-commit: 42571a36b6eac469ee94e305c7e7062306fde423
branch: fix/sys-cbsize-overflow-series

Re: [PATCH v2] common: cli_hush: fix console_buffer overflow on boot retry

[Sverdlin, Alexander]

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@student.rmit.edu.au>
> To: u-boot@lists.denx.de

The patch itself looks good to me and you can add my
Reviewed-by: Alexander Sverdlin <alexander.sverdlin@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@gmail.com>' != 'Signed-off-by: Ngo Luong Thanh Tra <S4210155@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

Re: [PATCH v2] common: cli_hush: fix console_buffer overflow on boot retry

[Simon Glass]

Hi,

On 2026-03-31T03:25:11, Ngo Luong Thanh Tra <ngotra27101996@gmail.com> wrote:
> common: cli_hush: fix console_buffer overflow on boot retry
>
> 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@student.rmit.edu.au>
> To: u-boot@lists.denx.de
> Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
> @@ -76,4 +76,27 @@
> +#define const_strcpy(d, s) ({                                                \
> +     BUILD_BUG_ON(__builtin_types_compatible_p(typeof(d), char *));  \

The macro checks that d is not char * but not const char * so if
someone passes a const char array, the first BUILD_BUG_ON passes
(since const char[] becomes const char *, not char *), but
__builtin_strcpy() would still fail because it cannot write to const
memory. The error message would be less clear though.

How about a check for const char * as well:

    BUILD_BUG_ON(__builtin_types_compatible_p(typeof(d), char *));
    BUILD_BUG_ON(__builtin_types_compatible_p(typeof(d), const char *));

Regards,
Simon

Re: [PATCH v2] common: cli_hush: fix console_buffer overflow on boot retry

[Rasmus Villemoes]

On Tue, Mar 31 2026, Ngo Luong Thanh Tra <ngotra27101996@gmail.com> 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@student.rmit.edu.au>
> To: u-boot@lists.denx.de
> ---
>
[snip]
> 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);						\
> +})
> +

+1 for the explanations you've added, that was very much my intention
that you should write something like that. (Note that d is also expanded
multiple times, but only _evalutated_ exactly once, since it appearing
inside typeof() or sizeof() does not cause it to be evaluated).

That said, I really think you should use the static_assert() version I
proposed. (Yes, there was a missing closing parenthesis that needed
fixing, other than that it seems to work).

That is evaluated much earlier by the compiler, and gives a more
to-the-point error message, instead of all the gunk that the
__attribute__((__error__)) implementation gives. For example, I just
tried adding

extern char *test_dst_p;
void test_const_strcpy(void)
{
	const_strcpy(test_dst_p, "foo");
}

to lib/string.c. With the BUILD_BUG_ON version, that gives

===
In file included from lib/string.c:21:
lib/string.c: In function ‘test_const_strcpy’:
include/linux/compiler.h:346:45: error: call to ‘__compiletime_assert_0’ declared with attribute error: BUILD_BUG_ON failed: __builtin_types_compatible_p(typeof(test_dst_p), char *)
  346 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
      |                                             ^
include/linux/compiler.h:327:25: note: in definition of macro ‘__compiletime_assert’
  327 |                         prefix ## suffix();                             \
      |                         ^~~~~~
include/linux/compiler.h:346:9: note: in expansion of macro ‘_compiletime_assert’
  346 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
      |         ^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:38:37: note: in expansion of macro ‘compiletime_assert’
   38 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
      |                                     ^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:49:9: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
   49 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |         ^~~~~~~~~~~~~~~~
include/linux/string.h:163:3: note: in expansion of macro ‘BUILD_BUG_ON’
  163 |   BUILD_BUG_ON(__builtin_types_compatible_p(typeof(d), char *)); \
      |   ^~~~~~~~~~~~
lib/string.c:31:9: note: in expansion of macro ‘const_strcpy’
   31 |         const_strcpy(test_dst_p, "foo");
      |         ^~~~~~~~~~~~
===

whereas with static_assert(), one gets

===
In file included from include/linux/string.h:6,
                 from lib/string.c:23:
lib/string.c: In function ‘test_const_strcpy’:
include/linux/build_bug.h:77:41: error: static assertion failed: "destination must be char array"
   77 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                         ^~~~~~~~~~~~~~
include/linux/build_bug.h:76:34: note: in expansion of macro ‘__static_assert’
   76 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
      |                                  ^~~~~~~~~~~~~~~
include/linux/string.h:156:3: note: in expansion of macro ‘static_assert’
  156 |   static_assert(__same_type(d, char[]), "destination must be char array"); \
      |   ^~~~~~~~~~~~~
lib/string.c:31:9: note: in expansion of macro ‘const_strcpy’
   31 |         const_strcpy(test_dst_p, "foo");
      |         ^~~~~~~~~~~~
===

That's much fewer lines of gunk, and moreover, the very first "error:"
line explains the problem, contrary to the rather opaque

  error: call to ‘__compiletime_assert_0’ declared with attribute error: BUILD_BUG_ON failed: __builtin_types_compatible_p(typeof(test_dst_p), char *)

And as others have pointed out, we really want to positively assert that
the destination has type char[], not merely that it is not char* - and
the documentation for __builtin_types_compatible_p explicitly says that
int[5] is compatible with int[], so even though we obviously want the
destination to have a known size, the type comparison to char[] is ok.

I also don't think this belongs in build_bug.h. It's a string operation,
so it's more natural in linux/string.h, where you must then add an
include of linux/build_bug.h to make that header self-contained. Or
maybe there's some other more appropriate header, but build_bug.h is not
it.

Another thing: You _can_ elide the type check for s as you've done as
the ""s"" trick _mostly_ does enforce it to be a string literal, but

(a) the error message one gets when there's a syntax error, e.g. when using
some const char* variable, is not as nice as what one can get from a
static_assert() ; something like

lib/string.c:34:38: error: expected ‘)’ before ‘test_src_p’
   34 |         const_strcpy(test_dst_array, test_src_p);

vs

include/linux/build_bug.h:77:41: error: static assertion failed: "source must be a string literal"
   77 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)


(b) there are evil ways to defeat the ""s"" trick:

    extern char test_dst_array[12];

    const_strcpy(test_dst_array, ""[0] + "foo bar baz frobble");

Since the s expression here both starts and ends with a string literal,
the concatenation trick works. However, ""[0] is simply another way of
spelling the integer 0 (because it's the nul terminator in the empty
string), and adding that to a string literal produces a value of type
"const char *", pointing to that string. So the sizeof("" s "") will
evaluate to 4 or 8, hence the size comparison will succeed, even though
the pointed-to string is actually longer than the destination array. So
this should not compile, but does, without the explicit type check.

Rasmus