[U-Boot] [PATCH 13/14] main: Make the execution path a little clearer in main.c

Simon Glass
2014-04-11T02:01:35Z

bootdelay_process() never returns in some circumstances, whichs makes the
control flow confusing. Change it so that the decision about how to execute
the boot command is made in the main_loop() code, so it is easier to follow.
Move CLI stuff to cli.c.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

common/autoboot.c | 81 ++++++++----------------------------------------------
common/cli.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
common/main.c | 12 +++++---
include/autoboot.h | 28 +++++++++++++++++--
include/cli.h | 33 ++++++++++++++++++++++
5 files changed, 145 insertions(+), 75 deletions(-)

diff --git a/common/autoboot.c b/common/autoboot.c
index 9843898..dc24cae 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -22,6 +22,9 @@ DECLARE_GLOBAL_DATA_PTR;
#define debug_bootkeys(fmt, args...) \
debug_cond(DEBUG_BOOTKEYS, fmt, ##args)

+/* Stored value of bootdelay, used by autoboot_command() */
+static int stored_bootdelay;
+
/***************************************************************************
* Watch for 'delay' seconds for autoboot stop or autoboot delay string.
* returns: 0 - no key string, allow autoboot 1 - got key string, abort
@@ -205,57 +208,9 @@ static int abortboot(int bootdelay)
#endif
}

-/*
- * Runs the given boot command securely. Specifically:
- * - Doesn't run the command with the shell (run_command or parse_string_outer),
- * since that's a lot of code surface that an attacker might exploit.
- * Because of this, we don't do any argument parsing--the secure boot command
- * has to be a full-fledged u-boot command.
- * - Doesn't check for keypresses before booting, since that could be a
- * security hole; also disables Ctrl-C.
- * - Doesn't allow the command to return.
- *
- * Upon any failures, this function will drop into an infinite loop after
- * printing the error message to console.
- */
-
-#if defined(CONFIG_OF_CONTROL)
-static void secure_boot_cmd(char *cmd)
-{
- cmd_tbl_t *cmdtp;
- int rc;
-
- if (!cmd) {
- printf("## Error: Secure boot command not specified\n");
- goto err;
- }
-
- /* Disable Ctrl-C just in case some command is used that checks it. */
- disable_ctrlc(1);
-
- /* Find the command directly. */
- cmdtp = find_cmd(cmd);
- if (!cmdtp) {
- printf("## Error: \"%s\" not defined\n", cmd);
- goto err;
- }
-
- /* Run the command, forcing no flags and faking argc and argv. */
- rc = (cmdtp->cmd)(cmdtp, 0, 1, &cmd);
-
- /* Shouldn't ever return from boot command. */
- printf("## Error: \"%s\" returned (code %d)\n", cmd, rc);
-
-err:
- /*
- * Not a whole lot to do here. Rebooting won't help much, since we'll
- * just end up right back here. Just loop.
- */
- hang();
-}
-
static void process_fdt_options(const void *blob)
{
+#if defined(CONFIG_OF_CONTROL)
ulong addr;

/* Add an env variable to point to a kernel payload, if available */
@@ -267,14 +222,11 @@ static void process_fdt_options(const void *blob)
addr = fdtdec_get_config_int(gd->fdt_blob, "rootdisk-offset", 0);
if (addr)
setenv_addr("rootaddr", (void *)(CONFIG_SYS_TEXT_BASE + addr));
-}
#endif /* CONFIG_OF_CONTROL */
+}

-void bootdelay_process(void)
+const char *bootdelay_process(void)
{
-#ifdef CONFIG_OF_CONTROL
- char *env;
-#endif
char *s;
int bootdelay;
#ifdef CONFIG_BOOTCOUNT_LIMIT
@@ -318,27 +270,18 @@ void bootdelay_process(void)
} else
#endif /* CONFIG_BOOTCOUNT_LIMIT */
s = getenv("bootcmd");
-#ifdef CONFIG_OF_CONTROL
- /* Allow the fdt to override the boot command */
- env = fdtdec_get_config_string(gd->fdt_blob, "bootcmd");
- if (env)
- s = env;

process_fdt_options(gd->fdt_blob);
+ stored_bootdelay = bootdelay;

- /*
- * If the bootsecure option was chosen, use secure_boot_cmd().
- * Always use 'env' in this case, since bootsecure requres that the
- * bootcmd was specified in the FDT too.
- */
- if (fdtdec_get_config_int(gd->fdt_blob, "bootsecure", 0))
- secure_boot_cmd(env);
-
-#endif /* CONFIG_OF_CONTROL */
+ return s;
+}

+void autoboot_command(const char *s)
+{
debug("### main_loop: bootcmd=\"%s\"\n", s ? s : "<UNDEFINED>");

- if (bootdelay != -1 && s && !abortboot(bootdelay)) {
+ if (stored_bootdelay != -1 && s && !abortboot(stored_bootdelay)) {
#if defined(CONFIG_AUTOBOOT_KEYED) && !defined(CONFIG_AUTOBOOT_KEYED_CTRLC)
int prev = disable_ctrlc(1); /* disable Control C checking */
#endif
diff --git a/common/cli.c b/common/cli.c
index 4ac9b3f..ea6bfb3 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -12,8 +12,11 @@
#include <common.h>
#include <cli.h>
#include <cli_hush.h>
+#include <fdtdec.h>
#include <malloc.h>

+DECLARE_GLOBAL_DATA_PTR;
+
/*
* Run a command using the selected parser.
*
@@ -105,6 +108,69 @@ int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
}
#endif

+#ifdef CONFIG_OF_CONTROL
+bool cli_process_fdt(const char **cmdp)
+{
+ /* Allow the fdt to override the boot command */
+ char *env = fdtdec_get_config_string(gd->fdt_blob, "bootcmd");
+ if (env)
+ *cmdp = env;
+ /*
+ * If the bootsecure option was chosen, use secure_boot_cmd().
+ * Always use 'env' in this case, since bootsecure requres that the
+ * bootcmd was specified in the FDT too.
+ */
+ return fdtdec_get_config_int(gd->fdt_blob, "bootsecure", 0) != 0;
+}
+
+/*
+ * Runs the given boot command securely. Specifically:
+ * - Doesn't run the command with the shell (run_command or parse_string_outer),
+ * since that's a lot of code surface that an attacker might exploit.
+ * Because of this, we don't do any argument parsing--the secure boot command
+ * has to be a full-fledged u-boot command.
+ * - Doesn't check for keypresses before booting, since that could be a
+ * security hole; also disables Ctrl-C.
+ * - Doesn't allow the command to return.
+ *
+ * Upon any failures, this function will drop into an infinite loop after
+ * printing the error message to console.
+ */
+void cli_secure_boot_cmd(const char *cmd)
+{
+ cmd_tbl_t *cmdtp;
+ int rc;
+
+ if (!cmd) {
+ printf("## Error: Secure boot command not specified\n");
+ goto err;
+ }
+
+ /* Disable Ctrl-C just in case some command is used that checks it. */
+ disable_ctrlc(1);
+
+ /* Find the command directly. */
+ cmdtp = find_cmd(cmd);
+ if (!cmdtp) {
+ printf("## Error: \"%s\" not defined\n", cmd);
+ goto err;
+ }
+
+ /* Run the command, forcing no flags and faking argc and argv. */
+ rc = (cmdtp->cmd)(cmdtp, 0, 1, (char **)&cmd);
+
+ /* Shouldn't ever return from boot command. */
+ printf("## Error: \"%s\" returned (code %d)\n", cmd, rc);
+
+err:
+ /*
+ * Not a whole lot to do here. Rebooting won't help much, since we'll
+ * just end up right back here. Just loop.
+ */
+ hang();
+}
+#endif /* CONFIG_OF_CONTROL */
+
void cli_loop(void)
{
#ifdef CONFIG_SYS_HUSH_PARSER
diff --git a/common/main.c b/common/main.c
index 12f6ac4..b0d3ea4 100644
--- a/common/main.c
+++ b/common/main.c
@@ -55,8 +55,11 @@ static void run_preboot_environment_command(void)
#endif /* CONFIG_PREBOOT */
}

+/* We come here after U-Boot is initialised and ready to process commands */
void main_loop(void)
{
+ const char *s;
+
bootstage_mark_name(BOOTSTAGE_ID_MAIN_LOOP, "main_loop");

modem_init();
@@ -72,10 +75,11 @@ void main_loop(void)
update_tftp(0UL);
#endif /* CONFIG_UPDATE_TFTP */

- bootdelay_process();
- /*
- * Main Loop for Monitor Command Processing
- */
+ s = bootdelay_process();
+ if (cli_process_fdt(&s))
+ cli_secure_boot_cmd(s);
+
+ autoboot_command(s);

cli_loop();
}
diff --git a/include/autoboot.h b/include/autoboot.h
index aaae4af..3a9059a 100644
--- a/include/autoboot.h
+++ b/include/autoboot.h
@@ -13,9 +13,33 @@
#define __AUTOBOOT_H

#ifdef CONFIG_BOOTDELAY
-void bootdelay_process(void);
+/**
+ * bootdelay_process() - process the bootd delay
+ *
+ * Process the boot delay, boot limit, then get the value of either
+ * bootcmd, failbootcmd or altbootcmd depending on the current state.
+ * Return this command so it can be executed.
+ *
+ * @return command to executed
+ */
+const char *bootdelay_process(void);
+
+/**
+ * autoboot_command() - run the autoboot command
+ *
+ * If enabled, run the autoboot command returned from bootdelay_process().
+ * Also do the CONFIG_MENUKEY processing if enabled.
+ *
+ * @cmd: Command to run
+ */
+void autoboot_command(const char *cmd);
#else
-static inline void bootdelay_process(void)
+static inline const char *bootdelay_process(void)
+{
+ return NULL;
+}
+
+static inline void autoboot_command(const char *s)
{
}
#endif
diff --git a/include/cli.h b/include/cli.h
index 5158976..6994262 100644
--- a/include/cli.h
+++ b/include/cli.h
@@ -100,6 +100,39 @@ int cli_readline_into_buffer(const char *const prompt, char *buffer,
*/
int cli_simple_parse_line(char *line, char *argv[]);

+#ifdef CONFIG_OF_CONTROL
+/**
+ * cli_process_fdt() - process the boot command from the FDT
+ *
+ * If bootcmmd is defined in the /config node of the FDT, we use that
+ * as the boot command. Further, if bootsecure is set to 1 (in the same
+ * node) then we return true, indicating that the command should be executed
+ * as securely as possible, avoiding the CLI parser.
+ *
+ * @cmdp: On entry, the command that will be executed if the FDT does
+ * not have a command. Returns the command to execute after
+ * checking the FDT.
+ * @return true to execute securely, else false
+ */
+bool cli_process_fdt(const char **cmdp);
+
+/** cli_secure_boot_cmd() - execute a command as securely as possible
+ *
+ * This avoids using the parser, thus executing the command with the
+ * smallest amount of code. Parameters are not supported.
+ */
+void cli_secure_boot_cmd(const char *cmd);
+#else
+static inline bool cli_process_fdt(const char **cmdp)
+{
+ return false;
+}
+
+static inline void cli_secure_boot_cmd(const char *cmd)
+{
+}
+#endif /* CONFIG_OF_CONTROL */
+
/**
* Go into the command loop
*
--
1.9.1.423.g4596e3a