main: Make the execution path a little clearer in main.c

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>

main: Make the execution path a little clearer in main.c
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>
Simon Glass · Tom Rini
1 parent c1bb2cd0b6
Showing 5 changed files with 145 additions and 75 deletions Side-by-side Diff
common/autoboot.c
common/cli.c
common/main.c
include/autoboot.h
include/cli.h
@@ -22,6 +22,9 @@
 #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 @@
 #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 @@
 	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 @@
 	} 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);
+	return s;
+}
  
-#endif /* CONFIG_OF_CONTROL */
-
+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
@@ -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.
  *
@@ -104,6 +107,69 @@
 	return 0;
 }
 #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)
 {
@@ -55,8 +55,11 @@
 #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");
  
 #ifndef CONFIG_SYS_GENERIC_BOARD
@@ -78,10 +81,11 @@
 	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();
 }
@@ -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
@@ -100,6 +100,39 @@
  */
 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
  *
...	...	@@ -22,6 +22,9 @@
22	22	#define debug_bootkeys(fmt, args...) \
23	23	debug_cond(DEBUG_BOOTKEYS, fmt, ##args)
24	24
	25	+/* Stored value of bootdelay, used by autoboot_command() */
	26	+static int stored_bootdelay;
	27	+
25	28	/***************************************************************************
26	29	* Watch for 'delay' seconds for autoboot stop or autoboot delay string.
27	30	* returns: 0 - no key string, allow autoboot 1 - got key string, abort
28	31
...	...	@@ -205,57 +208,9 @@
205	208	#endif
206	209	}
207	210
208		-/*
209		- * Runs the given boot command securely. Specifically:
210		- * - Doesn't run the command with the shell (run_command or parse_string_outer),
211		- * since that's a lot of code surface that an attacker might exploit.
212		- * Because of this, we don't do any argument parsing--the secure boot command
213		- * has to be a full-fledged u-boot command.
214		- * - Doesn't check for keypresses before booting, since that could be a
215		- * security hole; also disables Ctrl-C.
216		- * - Doesn't allow the command to return.
217		- *
218		- * Upon any failures, this function will drop into an infinite loop after
219		- * printing the error message to console.
220		- */
221		-
222		-#if defined(CONFIG_OF_CONTROL)
223		-static void secure_boot_cmd(char *cmd)
224		-{
225		- cmd_tbl_t *cmdtp;
226		- int rc;
227		-
228		- if (!cmd) {
229		- printf("## Error: Secure boot command not specified\n");
230		- goto err;
231		- }
232		-
233		- /* Disable Ctrl-C just in case some command is used that checks it. */
234		- disable_ctrlc(1);
235		-
236		- /* Find the command directly. */
237		- cmdtp = find_cmd(cmd);
238		- if (!cmdtp) {
239		- printf("## Error: \"%s\" not defined\n", cmd);
240		- goto err;
241		- }
242		-
243		- /* Run the command, forcing no flags and faking argc and argv. */
244		- rc = (cmdtp->cmd)(cmdtp, 0, 1, &cmd);
245		-
246		- /* Shouldn't ever return from boot command. */
247		- printf("## Error: \"%s\" returned (code %d)\n", cmd, rc);
248		-
249		-err:
250		- /*
251		- * Not a whole lot to do here. Rebooting won't help much, since we'll
252		- * just end up right back here. Just loop.
253		- */
254		- hang();
255		-}
256		-
257	211	static void process_fdt_options(const void *blob)
258	212	{
	213	+#if defined(CONFIG_OF_CONTROL)
259	214	ulong addr;
260	215
261	216	/* Add an env variable to point to a kernel payload, if available */
262	217
263	218
264	219
...	...	@@ -267,14 +222,11 @@
267	222	addr = fdtdec_get_config_int(gd->fdt_blob, "rootdisk-offset", 0);
268	223	if (addr)
269	224	setenv_addr("rootaddr", (void *)(CONFIG_SYS_TEXT_BASE + addr));
270		-}
271	225	#endif /* CONFIG_OF_CONTROL */
	226	+}
272	227
273		-void bootdelay_process(void)
	228	+const char *bootdelay_process(void)
274	229	{
275		-#ifdef CONFIG_OF_CONTROL
276		- char *env;
277		-#endif
278	230	char *s;
279	231	int bootdelay;
280	232	#ifdef CONFIG_BOOTCOUNT_LIMIT
281	233
282	234
283	235
284	236
...	...	@@ -318,27 +270,18 @@
318	270	} else
319	271	#endif /* CONFIG_BOOTCOUNT_LIMIT */
320	272	s = getenv("bootcmd");
321		-#ifdef CONFIG_OF_CONTROL
322		- /* Allow the fdt to override the boot command */
323		- env = fdtdec_get_config_string(gd->fdt_blob, "bootcmd");
324		- if (env)
325		- s = env;
326	273
327	274	process_fdt_options(gd->fdt_blob);
	275	+ stored_bootdelay = bootdelay;
328	276
329		- /*
330		- * If the bootsecure option was chosen, use secure_boot_cmd().
331		- * Always use 'env' in this case, since bootsecure requres that the
332		- * bootcmd was specified in the FDT too.
333		- */
334		- if (fdtdec_get_config_int(gd->fdt_blob, "bootsecure", 0))
335		- secure_boot_cmd(env);
	277	+ return s;
	278	+}
336	279
337		-#endif /* CONFIG_OF_CONTROL */
338		-
	280	+void autoboot_command(const char *s)
	281	+{
339	282	debug("### main_loop: bootcmd=\"%s\"\n", s ? s : "<UNDEFINED>");
340	283
341		- if (bootdelay != -1 && s && !abortboot(bootdelay)) {
	284	+ if (stored_bootdelay != -1 && s && !abortboot(stored_bootdelay)) {
342	285	#if defined(CONFIG_AUTOBOOT_KEYED) && !defined(CONFIG_AUTOBOOT_KEYED_CTRLC)
343	286	int prev = disable_ctrlc(1); /* disable Control C checking */
344	287	#endif
...	...	@@ -12,8 +12,11 @@
12	12	#include <common.h>
13	13	#include <cli.h>
14	14	#include <cli_hush.h>
	15	+#include <fdtdec.h>
15	16	#include <malloc.h>
16	17
	18	+DECLARE_GLOBAL_DATA_PTR;
	19	+
17	20	/*
18	21	* Run a command using the selected parser.
19	22	*
...	...	@@ -104,6 +107,69 @@
104	107	return 0;
105	108	}
106	109	#endif
	110	+
	111	+#ifdef CONFIG_OF_CONTROL
	112	+bool cli_process_fdt(const char **cmdp)
	113	+{
	114	+ /* Allow the fdt to override the boot command */
	115	+ char *env = fdtdec_get_config_string(gd->fdt_blob, "bootcmd");
	116	+ if (env)
	117	+ *cmdp = env;
	118	+ /*
	119	+ * If the bootsecure option was chosen, use secure_boot_cmd().
	120	+ * Always use 'env' in this case, since bootsecure requres that the
	121	+ * bootcmd was specified in the FDT too.
	122	+ */
	123	+ return fdtdec_get_config_int(gd->fdt_blob, "bootsecure", 0) != 0;
	124	+}
	125	+
	126	+/*
	127	+ * Runs the given boot command securely. Specifically:
	128	+ * - Doesn't run the command with the shell (run_command or parse_string_outer),
	129	+ * since that's a lot of code surface that an attacker might exploit.
	130	+ * Because of this, we don't do any argument parsing--the secure boot command
	131	+ * has to be a full-fledged u-boot command.
	132	+ * - Doesn't check for keypresses before booting, since that could be a
	133	+ * security hole; also disables Ctrl-C.
	134	+ * - Doesn't allow the command to return.
	135	+ *
	136	+ * Upon any failures, this function will drop into an infinite loop after
	137	+ * printing the error message to console.
	138	+ */
	139	+void cli_secure_boot_cmd(const char *cmd)
	140	+{
	141	+ cmd_tbl_t *cmdtp;
	142	+ int rc;
	143	+
	144	+ if (!cmd) {
	145	+ printf("## Error: Secure boot command not specified\n");
	146	+ goto err;
	147	+ }
	148	+
	149	+ /* Disable Ctrl-C just in case some command is used that checks it. */
	150	+ disable_ctrlc(1);
	151	+
	152	+ /* Find the command directly. */
	153	+ cmdtp = find_cmd(cmd);
	154	+ if (!cmdtp) {
	155	+ printf("## Error: \"%s\" not defined\n", cmd);
	156	+ goto err;
	157	+ }
	158	+
	159	+ /* Run the command, forcing no flags and faking argc and argv. */
	160	+ rc = (cmdtp->cmd)(cmdtp, 0, 1, (char **)&cmd);
	161	+
	162	+ /* Shouldn't ever return from boot command. */
	163	+ printf("## Error: \"%s\" returned (code %d)\n", cmd, rc);
	164	+
	165	+err:
	166	+ /*
	167	+ * Not a whole lot to do here. Rebooting won't help much, since we'll
	168	+ * just end up right back here. Just loop.
	169	+ */
	170	+ hang();
	171	+}
	172	+#endif /* CONFIG_OF_CONTROL */
107	173
108	174	void cli_loop(void)
109	175	{
...	...	@@ -55,8 +55,11 @@
55	55	#endif /* CONFIG_PREBOOT */
56	56	}
57	57
	58	+/* We come here after U-Boot is initialised and ready to process commands */
58	59	void main_loop(void)
59	60	{
	61	+ const char *s;
	62	+
60	63	bootstage_mark_name(BOOTSTAGE_ID_MAIN_LOOP, "main_loop");
61	64
62	65	#ifndef CONFIG_SYS_GENERIC_BOARD
...	...	@@ -78,10 +81,11 @@
78	81	update_tftp(0UL);
79	82	#endif /* CONFIG_UPDATE_TFTP */
80	83
81		- bootdelay_process();
82		- /*
83		- * Main Loop for Monitor Command Processing
84		- */
	84	+ s = bootdelay_process();
	85	+ if (cli_process_fdt(&s))
	86	+ cli_secure_boot_cmd(s);
	87	+
	88	+ autoboot_command(s);
85	89
86	90	cli_loop();
87	91	}
...	...	@@ -13,9 +13,33 @@
13	13	#define __AUTOBOOT_H
14	14
15	15	#ifdef CONFIG_BOOTDELAY
16		-void bootdelay_process(void);
	16	+/**
	17	+ * bootdelay_process() - process the bootd delay
	18	+ *
	19	+ * Process the boot delay, boot limit, then get the value of either
	20	+ * bootcmd, failbootcmd or altbootcmd depending on the current state.
	21	+ * Return this command so it can be executed.
	22	+ *
	23	+ * @return command to executed
	24	+ */
	25	+const char *bootdelay_process(void);
	26	+
	27	+/**
	28	+ * autoboot_command() - run the autoboot command
	29	+ *
	30	+ * If enabled, run the autoboot command returned from bootdelay_process().
	31	+ * Also do the CONFIG_MENUKEY processing if enabled.
	32	+ *
	33	+ * @cmd: Command to run
	34	+ */
	35	+void autoboot_command(const char *cmd);
17	36	#else
18		-static inline void bootdelay_process(void)
	37	+static inline const char *bootdelay_process(void)
	38	+{
	39	+ return NULL;
	40	+}
	41	+
	42	+static inline void autoboot_command(const char *s)
19	43	{
20	44	}
21	45	#endif
...	...	@@ -100,6 +100,39 @@
100	100	*/
101	101	int cli_simple_parse_line(char line, char argv[]);
102	102
	103	+#ifdef CONFIG_OF_CONTROL
	104	+/**
	105	+ * cli_process_fdt() - process the boot command from the FDT
	106	+ *
	107	+ * If bootcmmd is defined in the /config node of the FDT, we use that
	108	+ * as the boot command. Further, if bootsecure is set to 1 (in the same
	109	+ * node) then we return true, indicating that the command should be executed
	110	+ * as securely as possible, avoiding the CLI parser.
	111	+ *
	112	+ * @cmdp: On entry, the command that will be executed if the FDT does
	113	+ * not have a command. Returns the command to execute after
	114	+ * checking the FDT.
	115	+ * @return true to execute securely, else false
	116	+ */
	117	+bool cli_process_fdt(const char **cmdp);
	118	+
	119	+/** cli_secure_boot_cmd() - execute a command as securely as possible
	120	+ *
	121	+ * This avoids using the parser, thus executing the command with the
	122	+ * smallest amount of code. Parameters are not supported.
	123	+ */
	124	+void cli_secure_boot_cmd(const char *cmd);
	125	+#else
	126	+static inline bool cli_process_fdt(const char **cmdp)
	127	+{
	128	+ return false;
	129	+}
	130	+
	131	+static inline void cli_secure_boot_cmd(const char *cmd)
	132	+{
	133	+}
	134	+#endif /* CONFIG_OF_CONTROL */
	135	+
103	136	/**
104	137	* Go into the command loop
105	138	*