Patchwork [U-Boot,v2,04/10] arm: Add CONFIG_DELAY_ENVIRONMENT to delay environment loading

login
register
mail settings
Submitter Simon Glass
Date Nov. 30, 2012, 11:01 p.m.
Message ID <1354316484-23515-4-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/203077/
State Accepted, archived
Delegated to: Albert ARIBAUD
Headers show

Comments

Simon Glass - Nov. 30, 2012, 11:01 p.m.
This option delays loading of the environment until later, so that only the
default environment will be available to U-Boot.

This can address the security risk of untrusted data being used during boot.

Any time you load untrusted data you expose yourself to a bug in the
code. The attacker gets to choose the data so can sometimes carefully
craft it to exploit a bug. We try to avoid touching user-controlled
data during a verified boot unless strictly necessary. Since the
default environment is good enough in this case (or you would just
change it), this gets around the problem by just not loading the
environment.

When CONFIG_DELAY_ENVIRONMENT is defined, it is convenient to have a
run-time way of enabling loading of the environment. Add this to the
fdt as /config/delay-environment.

Note: This patch depends on http://patchwork.ozlabs.org/patch/194342/

Signed-off-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Update commit message to provide more detail

 README               |    9 +++++++++
 arch/arm/lib/board.c |   29 +++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 2 deletions(-)
Doug Anderson - Dec. 1, 2012, 1:24 a.m.
On Fri, Nov 30, 2012 at 3:01 PM, Simon Glass <sjg@chromium.org> wrote:
> This option delays loading of the environment until later, so that only the
> default environment will be available to U-Boot.
>
> This can address the security risk of untrusted data being used during boot.
>
> Any time you load untrusted data you expose yourself to a bug in the
> code. The attacker gets to choose the data so can sometimes carefully
> craft it to exploit a bug. We try to avoid touching user-controlled
> data during a verified boot unless strictly necessary. Since the
> default environment is good enough in this case (or you would just
> change it), this gets around the problem by just not loading the
> environment.
>
> When CONFIG_DELAY_ENVIRONMENT is defined, it is convenient to have a
> run-time way of enabling loading of the environment. Add this to the
> fdt as /config/delay-environment.
>
> Note: This patch depends on http://patchwork.ozlabs.org/patch/194342/
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Update commit message to provide more detail
>
>  README               |    9 +++++++++
>  arch/arm/lib/board.c |   29 +++++++++++++++++++++++++++--
>  2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/README b/README
> index b9a3685..d26ce5b 100644
> --- a/README
> +++ b/README
> @@ -2329,6 +2329,15 @@ CBFS (Coreboot Filesystem) support
>                 run-time determined information about the hardware to the
>                 environment.  These will be named board_name, board_rev.
>
> +               CONFIG_DELAY_ENVIRONMENT
> +
> +               Normally the environment is loaded when the board is
> +               intialised so that it is available to U-Boot. This inhibits
> +               that so that the environment is not available until
> +               explicitly loaded later by U-Boot code. With CONFIG_OF_CONTROL
> +               this is instead controlled by the value of
> +               /config/load-environment.
> +
>  - DataFlash Support:
>                 CONFIG_HAS_DATAFLASH
>
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 262a3ca..7d1927e 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -40,6 +40,7 @@
>
>  #include <common.h>
>  #include <command.h>
> +#include <environment.h>
>  #include <malloc.h>
>  #include <stdio_dev.h>
>  #include <version.h>
> @@ -476,7 +477,28 @@ static char *failed = "*** failed ***\n";
>  #endif
>
>  /*
> - ************************************************************************
> + * Tell if it's OK to load the environment early in boot.
> + *
> + * If CONFIG_OF_CONFIG is defined, we'll check with the FDT to see
> + * if this is OK (defaulting to saying it's not OK).
> + *
> + * NOTE: Loading the environment early can be a bad idea if security is
> + *       important, since no verification is done on the environment.
> + *
> + * @return 0 if environment should not be loaded, !=0 if it is ok to load
> + */
> +static int should_load_env(void)
> +{
> +#ifdef CONFIG_OF_CONTROL
> +       return fdtdec_get_config_int(gd->fdt_blob, "load-environment", 0);
> +#elif defined CONFIG_DELAY_ENVIRONMENT
> +       return 0;
> +#else
> +       return 1;
> +#endif
> +}
> +
> +/************************************************************************
>   *
>   * This is the next part if the initialization sequence: we are now
>   * running from RAM and have a "normal" C environment, i. e. global
> @@ -583,7 +605,10 @@ void board_init_r(gd_t *id, ulong dest_addr)
>  #endif
>
>         /* initialize environment */
> -       env_relocate();
> +       if (should_load_env())
> +               env_relocate();
> +       else
> +               set_default_env(NULL);
>
>  #if defined(CONFIG_CMD_PCI) || defined(CONFIG_PCI)
>         arm_pci_init();
> --
> 1.7.7.3
>

Reviewed-by: Doug Anderson <dianders@chromium.org>

Patch

diff --git a/README b/README
index b9a3685..d26ce5b 100644
--- a/README
+++ b/README
@@ -2329,6 +2329,15 @@  CBFS (Coreboot Filesystem) support
 		run-time determined information about the hardware to the
 		environment.  These will be named board_name, board_rev.
 
+		CONFIG_DELAY_ENVIRONMENT
+
+		Normally the environment is loaded when the board is
+		intialised so that it is available to U-Boot. This inhibits
+		that so that the environment is not available until
+		explicitly loaded later by U-Boot code. With CONFIG_OF_CONTROL
+		this is instead controlled by the value of
+		/config/load-environment.
+
 - DataFlash Support:
 		CONFIG_HAS_DATAFLASH
 
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 262a3ca..7d1927e 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -40,6 +40,7 @@ 
 
 #include <common.h>
 #include <command.h>
+#include <environment.h>
 #include <malloc.h>
 #include <stdio_dev.h>
 #include <version.h>
@@ -476,7 +477,28 @@  static char *failed = "*** failed ***\n";
 #endif
 
 /*
- ************************************************************************
+ * Tell if it's OK to load the environment early in boot.
+ *
+ * If CONFIG_OF_CONFIG is defined, we'll check with the FDT to see
+ * if this is OK (defaulting to saying it's not OK).
+ *
+ * NOTE: Loading the environment early can be a bad idea if security is
+ *       important, since no verification is done on the environment.
+ *
+ * @return 0 if environment should not be loaded, !=0 if it is ok to load
+ */
+static int should_load_env(void)
+{
+#ifdef CONFIG_OF_CONTROL
+	return fdtdec_get_config_int(gd->fdt_blob, "load-environment", 0);
+#elif defined CONFIG_DELAY_ENVIRONMENT
+	return 0;
+#else
+	return 1;
+#endif
+}
+
+/************************************************************************
  *
  * This is the next part if the initialization sequence: we are now
  * running from RAM and have a "normal" C environment, i. e. global
@@ -583,7 +605,10 @@  void board_init_r(gd_t *id, ulong dest_addr)
 #endif
 
 	/* initialize environment */
-	env_relocate();
+	if (should_load_env())
+		env_relocate();
+	else
+		set_default_env(NULL);
 
 #if defined(CONFIG_CMD_PCI) || defined(CONFIG_PCI)
 	arm_pci_init();