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 *