All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] board: samsung: fix set_board_info() board_name buffer overflow
@ 2026-03-28  6:01 Ngo Luong Thanh Tra
  2026-03-28  6:01 ` [PATCH 2/3] board: toradex: fix tdx-cfg-block prompt " Ngo Luong Thanh Tra
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Ngo Luong Thanh Tra @ 2026-03-28  6:01 UTC (permalink / raw)
  To: u-boot; +Cc: Ngo Luong Thanh Tra, Minkyu Kang, Przemyslaw Marczak, Tom Rini

Replace unbounded sprintf() with snprintf() using sizeof(info) as
the bound when constructing the board_name string from bdname and
bdtype. The previous call had no size limit and could overflow the
64-byte stack buffer if the concatenated string exceeded 63 bytes.

Fixes: c9c36bf56e4c ("samsung: misc: use board specific functions to set env board info")
Signed-off-by: Ngo Luong Thanh Tra <S4210155@student.rmit.edu.au>
To: u-boot@lists.denx.de
---

 board/samsung/common/misc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c
index c134a9d70e2..6718f607875 100644
--- a/board/samsung/common/misc.c
+++ b/board/samsung/common/misc.c
@@ -104,7 +104,7 @@ void set_board_info(void)
 	if (!bdtype)
 		bdtype = "";
 
-	sprintf(info, "%s%s", bdname, bdtype);
+	snprintf(info, sizeof(info), "%s%s", bdname, bdtype);
 	env_set("board_name", info);
 #endif
 	snprintf(info, ARRAY_SIZE(info),  "%s%x-%s%s.dtb",
-- 
2.53.0

base-commit: c704af3c8b0f37929bce8c2a4bba27d6e89919c7
branch: fix/sys-cbsize-overflow-series

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] board: toradex: fix tdx-cfg-block prompt buffer overflow
  2026-03-28  6:01 [PATCH 1/3] board: samsung: fix set_board_info() board_name buffer overflow Ngo Luong Thanh Tra
@ 2026-03-28  6:01 ` Ngo Luong Thanh Tra
  2026-03-28  6:01 ` [PATCH 3/3] common: cli_hush: fix console_buffer overflow on boot retry Ngo Luong Thanh Tra
  2026-04-14  2:09 ` (subset) [PATCH 1/3] board: samsung: fix set_board_info() board_name buffer overflow Tom Rini
  2 siblings, 0 replies; 5+ messages in thread
From: Ngo Luong Thanh Tra @ 2026-03-28  6:01 UTC (permalink / raw)
  To: u-boot
  Cc: Ngo Luong Thanh Tra, Emanuele Ghidoli, Francesco Dolcini,
	Tom Rini, Vitor Soares

Replace unbounded sprintf() with snprintf() using sizeof(message)
as the bound for all prompt string assignments in
get_cfgblock_interactive(), get_cfgblock_carrier_interactive(),
do_cfgblock_carrier_create() and do_cfgblock_create(). The
previous calls had no size limit and could overflow the
CONFIG_SYS_CBSIZE-sized stack buffer if SYS_CBSIZE was configured
smaller than the longest prompt string (71 bytes).

Fixes: 8b6dc5d3943c ("toradex: tdx-cfg-block: Cleanup interactive cfg block creation")
Signed-off-by: Ngo Luong Thanh Tra <S4210155@student.rmit.edu.au>
To: u-boot@lists.denx.de
---

 board/toradex/common/tdx-cfg-block.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/board/toradex/common/tdx-cfg-block.c b/board/toradex/common/tdx-cfg-block.c
index 0fc3759695f..d75a6754c68 100644
--- a/board/toradex/common/tdx-cfg-block.c
+++ b/board/toradex/common/tdx-cfg-block.c
@@ -508,7 +508,7 @@ static int get_cfgblock_interactive(void)
 			       toradex_modules[i].name);
 	}
 
-	sprintf(message, "Enter the module ID: ");
+	snprintf(message, sizeof(message), "Enter the module ID: ");
 	len = cli_readline(message);
 
 	prodid = dectoul(console_buffer, NULL);
@@ -521,7 +521,8 @@ static int get_cfgblock_interactive(void)
 
 	len = 0;
 	while (len < 4) {
-		sprintf(message, "Enter the module version (e.g. V1.1B or V1.1#26): V");
+		snprintf(message, sizeof(message),
+			 "Enter the module version (e.g. V1.1B or V1.1#26): V");
 		len = cli_readline(message);
 	}
 
@@ -535,7 +536,7 @@ static int get_cfgblock_interactive(void)
 	}
 
 	while (len < 8) {
-		sprintf(message, "Enter module serial number: ");
+		snprintf(message, sizeof(message), "Enter module serial number: ");
 		len = cli_readline(message);
 	}
 
@@ -744,12 +745,13 @@ static int get_cfgblock_carrier_interactive(void)
 		       toradex_carrier_boards[i].name,
 		       toradex_carrier_boards[i].pid4);
 
-	sprintf(message, "Choose your carrier board (provide ID): ");
+	snprintf(message, sizeof(message), "Choose your carrier board (provide ID): ");
 	len = cli_readline(message);
 	tdx_car_hw_tag.prodid = dectoul(console_buffer, NULL);
 
 	do {
-		sprintf(message, "Enter carrier board version (e.g. V1.1B or V1.1#26): V");
+		snprintf(message, sizeof(message),
+			 "Enter carrier board version (e.g. V1.1B or V1.1#26): V");
 		len = cli_readline(message);
 	} while (len < 4);
 
@@ -763,7 +765,7 @@ static int get_cfgblock_carrier_interactive(void)
 	}
 
 	while (len < 8) {
-		sprintf(message, "Enter carrier board serial number: ");
+		snprintf(message, sizeof(message), "Enter carrier board serial number: ");
 		len = cli_readline(message);
 	}
 
@@ -799,7 +801,8 @@ static int do_cfgblock_carrier_create(struct cmd_tbl *cmdtp, int flag, int argc,
 	if (valid_cfgblock_carrier && !force_overwrite) {
 		char message[CONFIG_SYS_CBSIZE];
 
-		sprintf(message, "A valid Toradex Carrier config block is present, still recreate? [y/N] ");
+		snprintf(message, sizeof(message),
+			 "A valid Toradex Carrier config block is present, still recreate? [y/N] ");
 
 		if (!cli_readline(message))
 			goto out;
@@ -907,8 +910,8 @@ static int do_cfgblock_create(struct cmd_tbl *cmdtp, int flag, int argc,
 		if (!force_overwrite) {
 			char message[CONFIG_SYS_CBSIZE];
 
-			sprintf(message,
-				"A valid Toradex config block is present, still recreate? [y/N] ");
+			snprintf(message, sizeof(message),
+				 "A valid Toradex config block is present, still recreate? [y/N] ");
 
 			if (!cli_readline(message))
 				goto out;
-- 
2.53.0

base-commit: c704af3c8b0f37929bce8c2a4bba27d6e89919c7
branch: fix/sys-cbsize-overflow-series

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] common: cli_hush: fix console_buffer overflow on boot retry
  2026-03-28  6:01 [PATCH 1/3] board: samsung: fix set_board_info() board_name buffer overflow Ngo Luong Thanh Tra
  2026-03-28  6:01 ` [PATCH 2/3] board: toradex: fix tdx-cfg-block prompt " Ngo Luong Thanh Tra
@ 2026-03-28  6:01 ` Ngo Luong Thanh Tra
  2026-03-30 12:59   ` Rasmus Villemoes
  2026-04-14  2:09 ` (subset) [PATCH 1/3] board: samsung: fix set_board_info() board_name buffer overflow Tom Rini
  2 siblings, 1 reply; 5+ messages in thread
From: Ngo Luong Thanh Tra @ 2026-03-28  6:01 UTC (permalink / raw)
  To: u-boot; +Cc: Ngo Luong Thanh Tra, Casey Connolly, Tom Rini

Replace strcpy() with strlcpy() when injecting the boot retry
command into console_buffer. Add a BUILD_BUG_ON() to catch at
compile time any configuration where CONFIG_SYS_CBSIZE is smaller
than the retry command string, and use a named constant for the
command so that the size check stays in sync if the string is
ever changed.

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
---

 common/cli_hush.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/common/cli_hush.c b/common/cli_hush.c
index 7bd6943d3ed..6141c2959df 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,10 @@ 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");
+	static const char retry_cmd[] = "run bootcmd\n";
+
+	BUILD_BUG_ON(sizeof(retry_cmd) - 1 > CONFIG_SYS_CBSIZE);
+	strlcpy(console_buffer, retry_cmd, sizeof(console_buffer));
 # else
 #	error "This only works with CONFIG_RESET_TO_RETRY or CONFIG_BOOT_RETRY_COMMAND enabled"
 #  endif
-- 
2.53.0

base-commit: c704af3c8b0f37929bce8c2a4bba27d6e89919c7
branch: fix/sys-cbsize-overflow-series

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] common: cli_hush: fix console_buffer overflow on boot retry
  2026-03-28  6:01 ` [PATCH 3/3] common: cli_hush: fix console_buffer overflow on boot retry Ngo Luong Thanh Tra
@ 2026-03-30 12:59   ` Rasmus Villemoes
  0 siblings, 0 replies; 5+ messages in thread
From: Rasmus Villemoes @ 2026-03-30 12:59 UTC (permalink / raw)
  To: Ngo Luong Thanh Tra; +Cc: u-boot, Ngo Luong Thanh Tra, Casey Connolly, Tom Rini

On Sat, Mar 28 2026, Ngo Luong Thanh Tra <ngotra27101996@gmail.com> wrote:

> Replace strcpy() with strlcpy() when injecting the boot retry
> command into console_buffer. Add a BUILD_BUG_ON() to catch at
> compile time any configuration where CONFIG_SYS_CBSIZE is smaller
> than the retry command string, and use a named constant for the
> command so that the size check stays in sync if the string is
> ever changed.
>
> 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
> ---
>
>  common/cli_hush.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/common/cli_hush.c b/common/cli_hush.c
> index 7bd6943d3ed..6141c2959df 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,10 @@ 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");
> +	static const char retry_cmd[] = "run bootcmd\n";
> +
> +	BUILD_BUG_ON(sizeof(retry_cmd) - 1 > CONFIG_SYS_CBSIZE);
> +	strlcpy(console_buffer, retry_cmd, sizeof(console_buffer));

Have you compiled this?

The declaration of console_buffer in include/console.h does not include
the size, so you should get a build error like

  error: invalid application of 'sizeof' to incomplete type 'char[]'

And exactly because that declaration doesn't include the size, the -1
and the comparison to CONFIG_SYS_CBSIZE looks rather fishy.

If anything, one should start by making the size of console_buffer part
of the declaration, so that users such as here could actually do
sizeof(console_buffer), and then one should not use or need to know that
the size if defined in terms of (but not exactly equal to)
CONFIG_SYS_CBSIZE.

Also, I generally think that this whole "must use strlcpy because
safer!" is broken when everything in sight are compile-time
constants. Because the compiler knows about strcpy(), so it can optimize
a strcpy() with a literal as source into a sequence of a few immediate
stores, which is often smaller code than emitting the string literal to
.rodata.str and emitting an actual strcpy() call with all the register
save/restore that requires. It knows nothing about strlcpy().

Unfortunately, U-Boot builds with -Wno-array-bounds, so just declaring
console_buffer with its actual size is not enough to trigger a build
error with the current strcpy(). But if you want to improve stuff in
this area, do something like creating a const_strcpy() helper macro
which will enforce that

  (a) The source is a string literal
  (b) The destination is a char array of known size
  (c) Makes it a build-time error if it doesn't fit
  (d) Uses __builtin_strcpy(dst, src) to tell the compiler that this
  really is just a strcpy(), even if -fno-builtin is in effect, and let
  the compiler optimize as it sees fit - including eliminating the whole
  thing as dead stores if it sees that the destination is not actually
  used.

Something like

#define const_strcpy(d, s) ({ \
  static_assert(__same_type(d, char[]), "destination must be char array"); \
  static_assert(__same_type(s, const char[], "source must be a string literal"); \
  static_assert(sizeof(d) >= sizeof("" s ""), "source does not fit in destination"); \
  __builtin_strcpy(d, s); \
})

Rasmus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: (subset) [PATCH 1/3] board: samsung: fix set_board_info() board_name buffer overflow
  2026-03-28  6:01 [PATCH 1/3] board: samsung: fix set_board_info() board_name buffer overflow Ngo Luong Thanh Tra
  2026-03-28  6:01 ` [PATCH 2/3] board: toradex: fix tdx-cfg-block prompt " Ngo Luong Thanh Tra
  2026-03-28  6:01 ` [PATCH 3/3] common: cli_hush: fix console_buffer overflow on boot retry Ngo Luong Thanh Tra
@ 2026-04-14  2:09 ` Tom Rini
  2 siblings, 0 replies; 5+ messages in thread
From: Tom Rini @ 2026-04-14  2:09 UTC (permalink / raw)
  To: u-boot, Ngo Luong Thanh Tra
  Cc: Ngo Luong Thanh Tra, Minkyu Kang, Przemyslaw Marczak

On Sat, 28 Mar 2026 13:01:31 +0700, Ngo Luong Thanh Tra wrote:

> Replace unbounded sprintf() with snprintf() using sizeof(info) as
> the bound when constructing the board_name string from bdname and
> bdtype. The previous call had no size limit and could overflow the
> 64-byte stack buffer if the concatenated string exceeded 63 bytes.
> 
> 

Applied to u-boot/master, thanks!

[1/3] board: samsung: fix set_board_info() board_name buffer overflow
      commit: e228b6a50418e261ee09fa326464935acdabe610
[2/3] board: toradex: fix tdx-cfg-block prompt buffer overflow
      commit: 564e180d701f3946e6adc7895f2524728b985f03
-- 
Tom


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-14  2:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-28  6:01 [PATCH 1/3] board: samsung: fix set_board_info() board_name buffer overflow Ngo Luong Thanh Tra
2026-03-28  6:01 ` [PATCH 2/3] board: toradex: fix tdx-cfg-block prompt " Ngo Luong Thanh Tra
2026-03-28  6:01 ` [PATCH 3/3] common: cli_hush: fix console_buffer overflow on boot retry Ngo Luong Thanh Tra
2026-03-30 12:59   ` Rasmus Villemoes
2026-04-14  2:09 ` (subset) [PATCH 1/3] board: samsung: fix set_board_info() board_name buffer overflow Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.