[U-Boot] [PATCH 3/4] bootm: Avoid 256-byte overflow in fixup_silent_linux()

[Doug Anderson]

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.

Signed-off-by: Doug Anderson <dianders at chromium.org>
---
 common/cmd_bootm.c |  125 ++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 106 insertions(+), 19 deletions(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index ece1b9a..f426e2f 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -26,6 +26,7 @@
  * Boot support
  */
 #include <common.h>
+#include <cmdline.h>
 #include <watchdog.h>
 #include <command.h>
 #include <image.h>
@@ -1200,36 +1201,122 @@ U_BOOT_CMD(
 /* helper routines */
 /*******************************************************************/
 #ifdef CONFIG_SILENT_CONSOLE
+
+/**
+ * Remove "console=blah" and from cmdline, replace w/ "console=".
+ *
+ * This has the effect of telling Linux that we'd like it to have a silent
+ * console.
+ *
+ * @param cmdline	The original commanjd line.
+ * @return The new command line, which has been allocated with malloc().
+ *	   Might be NULL if we ran out of memory.
+ */
+static char *do_fixup_silent_linux(const char *cmdline)
+{
+	char *buf;
+	int bufsize;
+	int did_remove;
+
+	if (!cmdline)
+		cmdline = "";
+
+	/*
+	 * Allocate enough space for:
+	 * - a copy of the command line
+	 * - a space
+	 * - a blank "console=" argument
+	 * - the '\0'
+	 *
+	 * ...we might not need all this space, but it's OK to overallocate a
+	 * little.
+	 */
+	bufsize = strlen(cmdline) + 1 + sizeof("console=");
+	buf = malloc(bufsize);
+	if (!buf) {
+		debug("WARNING: malloc failed in fixup_silent_linux\n");
+		return NULL;
+	}
+
+	strcpy(buf, cmdline);
+	do {
+		did_remove  = remove_cmdline_param(buf, "console");
+	} while (did_remove);
+	add_cmdline_param(buf, "console=", bufsize);
+
+	return buf;
+}
+
 static void fixup_silent_linux(void)
 {
-	char buf[256], *start, *end;
-	char *cmdline = getenv("bootargs");
+	char *buf;
+	const char *cmdline = getenv("bootargs");
 
 	/* Only fix cmdline when requested */
 	if (!(gd->flags & GD_FLG_SILENT))
 		return;
 
 	debug("before silent fix-up: %s\n", cmdline);
-	if (cmdline) {
-		start = strstr(cmdline, "console=");
-		if (start) {
-			end = strchr(start, ' ');
-			strncpy(buf, cmdline, (start - cmdline + 8));
-			if (end)
-				strcpy(buf + (start - cmdline + 8), end);
-			else
-				buf[start - cmdline + 8] = '\0';
-		} else {
-			strcpy(buf, cmdline);
-			strcat(buf, " console=");
-		}
-	} else {
-		strcpy(buf, "console=");
+
+	buf = do_fixup_silent_linux(cmdline);
+	if (buf) {
+		setenv("bootargs", buf);
+		debug("after silent fix-up: %s\n", buf);
+		free(buf);
 	}
+}
 
-	setenv("bootargs", buf);
-	debug("after silent fix-up: %s\n", buf);
+/**
+ * Unit tests for do_fixup_silent_linux().
+ *
+ * At the moment, there's no easy way to run this other than to copy it (and
+ * do_fixup_silent_linux) to another file.
+ */
+#ifdef RUN_UNITTESTS
+void do_fixup_silent_linux_unittest(void)
+{
+	char *original_str;
+	char *expected_str;
+	char *result;
+
+	/* Simple case first, as an example */
+	original_str = "console=ttyS0,115200n8 root=/dev/mmcblk0p3 rootwait ro";
+	expected_str = "root=/dev/mmcblk0p3 rootwait ro console=";
+	result = do_fixup_silent_linux(original_str);
+	assert(strcmp(result, expected_str) == 0);
+	free(result);
+
+	/* Null cases next */
+	original_str = NULL;
+	expected_str = "console=";
+	result = do_fixup_silent_linux(original_str);
+	assert(strcmp(result, expected_str) == 0);
+	free(result);
+
+	original_str = "";
+	expected_str = "console=";
+	result = do_fixup_silent_linux(original_str);
+	assert(strcmp(result, expected_str) == 0);
+	free(result);
+
+	/* Throw console= at the end */
+	original_str = "root=/dev/mmcblk0p3 rootwait ro console=ttyS0,115200n8";
+	expected_str = "root=/dev/mmcblk0p3 rootwait ro console=";
+	result = do_fixup_silent_linux(original_str);
+	assert(strcmp(result, expected_str) == 0);
+	free(result);
+
+	/* Something non-NULL with no "console=" */
+	original_str = "root=/dev/mmcblk0p3 rootwait ro";
+	expected_str = "root=/dev/mmcblk0p3 rootwait ro console=";
+	result = do_fixup_silent_linux(original_str);
+	assert(strcmp(result, expected_str) == 0);
+	free(result);
+
+	debug("do_fixup_silent_linux_unittest: pass\n");
 }
+#endif /* RUN_UNITTESTS */
+
 #endif /* CONFIG_SILENT_CONSOLE */
 
 
-- 
1.7.3.1