All of lore.kernel.org
 help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 3/4] bootm: Avoid 256-byte overflow in fixup_silent_linux()
Date: Wed, 19 Oct 2011 15:30:58 -0700	[thread overview]
Message-ID: <1319063459-4804-4-git-send-email-dianders@chromium.org> (raw)
In-Reply-To: <1319063459-4804-1-git-send-email-dianders@chromium.org>

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

  parent reply	other threads:[~2011-10-19 22:30 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-19 22:30 [U-Boot] [PATCH 0/4] Fix fixup_silent_linux() buffer overrun Doug Anderson
2011-10-19 22:30 ` [U-Boot] [PATCH 1/4] cmdline: Add linux command line munging tools Doug Anderson
2011-10-19 22:46   ` Mike Frysinger
2011-10-20  1:23     ` Doug Anderson
2011-10-19 22:52   ` Mike Frysinger
2011-10-20  1:07     ` Doug Anderson
2011-10-20  1:37       ` Mike Frysinger
2011-10-20 14:36   ` Wolfgang Denk
2011-10-20 17:06     ` Doug Anderson
2011-10-20 17:15       ` Mike Frysinger
2011-10-20 18:23         ` Doug Anderson
2011-10-20 19:33           ` Wolfgang Denk
2011-10-20 19:03       ` Wolfgang Denk
2011-10-21  5:09         ` Doug Anderson
2011-10-19 22:30 ` [U-Boot] [PATCH 2/4] cosmetic: Fixup fixup_silent_linux() for checkpatch Doug Anderson
2011-10-20 14:38   ` Wolfgang Denk
2011-10-19 22:30 ` Doug Anderson [this message]
2011-10-19 22:51   ` [U-Boot] [PATCH 3/4] bootm: Avoid 256-byte overflow in fixup_silent_linux() Mike Frysinger
2011-10-20 14:40   ` Wolfgang Denk
2011-10-20 17:54     ` [U-Boot] [PATCH v2] " Doug Anderson
2012-01-10 22:28       ` Wolfgang Denk
2012-01-10 22:51         ` Doug Anderson
2012-01-10 23:31           ` Mike Frysinger
2012-01-10 23:30         ` Mike Frysinger
2012-01-11 18:19   ` Doug Anderson
2012-01-15  1:32     ` Mike Frysinger
2012-01-17 19:16     ` [U-Boot] [PATCH v3] " Doug Anderson
2012-01-17 19:27       ` Mike Frysinger
2012-01-17 19:33         ` Doug Anderson
2012-01-17 19:37     ` [U-Boot] [PATCH v4] " Doug Anderson
2012-01-17 19:55       ` Mike Frysinger
2013-05-22 14:59       ` [U-Boot] [U-Boot, " Tom Rini
2011-10-19 22:30 ` [U-Boot] [PATCH 4/4] bootm: Add earlyprintk to fixup_silent_linux Doug Anderson
2011-10-19 22:35   ` Mike Frysinger
2011-10-19 22:46     ` Doug Anderson
2011-10-19 23:11       ` Mike Frysinger
2011-10-20 14:42   ` Wolfgang Denk
2011-10-20 17:35     ` Doug Anderson
2011-10-20 19:26       ` Wolfgang Denk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1319063459-4804-4-git-send-email-dianders@chromium.org \
    --to=dianders@chromium.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.