[U-Boot,v4] bootm: Avoid 256-byte overflow in fixup_silent_linux()

Submitted by Doug Anderson on Jan. 17, 2012, 7:37 p.m.

Details

Message ID 1326829061-4682-1-git-send-email-dianders@chromium.org
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Doug Anderson Jan. 17, 2012, 7:37 p.m.
This makes fixup_silent_linux() use malloc() to allocate its
working space, meaning that our maximum kernel command line
should only be limited by malloc().  Previously it was silently
overflowing the stack.

Note that nothing about this change increases the kernel's maximum
command line length.  If you have a command line that is >256
bytes it's up to you to make sure that kernel can handle it.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v2:
- Tried to trim down to just the minimum changes needed with
no extra helper code.

Changes in v3:
- Took Mike Frysinger's suggestion of removing strdup()

Changes in v4:
- Added in const

 common/cmd_bootm.c |   41 +++++++++++++++++++++++++++++------------
 1 files changed, 29 insertions(+), 12 deletions(-)

Comments

Mike Frysinger Jan. 17, 2012, 7:55 p.m.
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
Tom Rini May 22, 2013, 2:59 p.m.
On Tue, Jan 17, 2012 at 09:37:41AM -0000, Doug Anderson wrote:

> This makes fixup_silent_linux() use malloc() to allocate its
> working space, meaning that our maximum kernel command line
> should only be limited by malloc().  Previously it was silently
> overflowing the stack.
> 
> Note that nothing about this change increases the kernel's maximum
> command line length.  If you have a command line that is >256
> bytes it's up to you to make sure that kernel can handle it.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Mike Frysinger <vapier@gentoo.org>

Applied to u-boot/master, thanks!

Patch hide | download patch | download mbox

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d5745b1..0a5ac81 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -1229,9 +1229,14 @@  U_BOOT_CMD(
 /* helper routines */
 /*******************************************************************/
 #ifdef CONFIG_SILENT_CONSOLE
+
+#define CONSOLE_ARG     "console="
+#define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1)
+
 static void fixup_silent_linux(void)
 {
-	char buf[256], *start, *end;
+	char *buf;
+	const char *env_val;
 	char *cmdline = getenv("bootargs");
 
 	/* Only fix cmdline when requested */
@@ -1239,25 +1244,37 @@  static void fixup_silent_linux(void)
 		return;
 
 	debug("before silent fix-up: %s\n", cmdline);
-	if (cmdline) {
-		start = strstr(cmdline, "console=");
+	if (cmdline && (cmdline[0] != '\0')) {
+		char *start = strstr(cmdline, CONSOLE_ARG);
+
+		/* Allocate space for maximum possible new command line */
+		buf = malloc(strlen(cmdline) + 1 + CONSOLE_ARG_LEN + 1);
+		if (!buf) {
+			debug("%s: out of memory\n", __func__);
+			return;
+		}
+
 		if (start) {
-			end = strchr(start, ' ');
-			strncpy(buf, cmdline, (start - cmdline + 8));
+			char *end = strchr(start, ' ');
+			int num_start_bytes = start - cmdline + CONSOLE_ARG_LEN;
+
+			strncpy(buf, cmdline, num_start_bytes);
 			if (end)
-				strcpy(buf + (start - cmdline + 8), end);
+				strcpy(buf + num_start_bytes, end);
 			else
-				buf[start - cmdline + 8] = '\0';
+				buf[num_start_bytes] = '\0';
 		} else {
-			strcpy(buf, cmdline);
-			strcat(buf, " console=");
+			sprintf(buf, "%s %s", cmdline, CONSOLE_ARG);
 		}
+		env_val = buf;
 	} else {
-		strcpy(buf, "console=");
+		buf = NULL;
+		env_val = CONSOLE_ARG;
 	}
 
-	setenv("bootargs", buf);
-	debug("after silent fix-up: %s\n", buf);
+	setenv("bootargs", env_val);
+	debug("after silent fix-up: %s\n", env_val);
+	free(buf);
 }
 #endif /* CONFIG_SILENT_CONSOLE */