Re: [U-Boot] [PATCH 05/10] arm: Add CONFIG_DELAY_ENVIRONMENT to delay environment loading

2013-01-18 Thread Lucas Stach
Am Donnerstag, den 01.11.2012, 16:42 -0700 schrieb Simon Glass:
 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.
 
 When CONFIG_DELAY_ENVIRONMENT is defined, it is convenient to have a
 run-time way of enabling loadinlg of the environment. Add this to the
 fdt as /config/delay-environment.
 
It's really unfortunate to only realize this after the final release of
v2013.01 as I haven't tested the -rc3, but this breaks environment for
almost all Tegra boards. I haven't checked all of them, but the ones I
looked at have CONFIG_OF_CONTROL defined, but no load-environment node
in the FDT.

So they're all going straight into secure boot mode, because of the
bogus standard value of not allowing env to load, which is probably not
what most people want.

Regards,
Lucas
 ---
[...]
  /*
 - 
 + * 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
 +}
 +
[...]

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 05/10] arm: Add CONFIG_DELAY_ENVIRONMENT to delay environment loading

2013-01-18 Thread Simon Glass
Hi Lucas,

On Fri, Jan 18, 2013 at 4:06 AM, Lucas Stach d...@lynxeye.de wrote:
 Am Donnerstag, den 01.11.2012, 16:42 -0700 schrieb Simon Glass:
 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.

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

 It's really unfortunate to only realize this after the final release of
 v2013.01 as I haven't tested the -rc3, but this breaks environment for
 almost all Tegra boards. I haven't checked all of them, but the ones I
 looked at have CONFIG_OF_CONTROL defined, but no load-environment node
 in the FDT.

 So they're all going straight into secure boot mode, because of the
 bogus standard value of not allowing env to load, which is probably not
 what most people want.

Hmmm yes I think you are right - the value would be better the other
way around. I will create a patch for this and see what people think.
I have tended to create my own FDT file but I'm sure many will not.

Regards,
Simon


 Regards,
 Lucas
 ---
 [...]
  /*
 - 
 + * 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
 +}
 +
 [...]

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 05/10] arm: Add CONFIG_DELAY_ENVIRONMENT to delay environment loading

2012-11-06 Thread Simon Glass
Hi Wolfgang,

On Sat, Nov 3, 2012 at 5:30 AM, Wolfgang Denk w...@denx.de wrote:
 Dear Simon Glass,

 In message 1351813330-23741-5-git-send-email-...@chromium.org you 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.

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

 Please explain what exactly this is good for, or which exact security
 risks this is supposed to fix.

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.


 As is, I strongly tend to NAK this.

 Best regards,

 Wolfgang Denk

Regards,
Simon


 --
 DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
 HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
 Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
 In the beginning, there was nothing, which exploded.
 - Terry Pratchett, _Lords and Ladies_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 05/10] arm: Add CONFIG_DELAY_ENVIRONMENT to delay environment loading

2012-11-03 Thread Wolfgang Denk
Dear Simon Glass,

In message 1351813330-23741-5-git-send-email-...@chromium.org you 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.
 
 When CONFIG_DELAY_ENVIRONMENT is defined, it is convenient to have a
 run-time way of enabling loadinlg of the environment. Add this to the
 fdt as /config/delay-environment.

Please explain what exactly this is good for, or which exact security
risks this is supposed to fix.

As is, I strongly tend to NAK this.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
In the beginning, there was nothing, which exploded.
- Terry Pratchett, _Lords and Ladies_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 05/10] arm: Add CONFIG_DELAY_ENVIRONMENT to delay environment loading

2012-11-01 Thread Simon Glass
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.

When CONFIG_DELAY_ENVIRONMENT is defined, it is convenient to have a
run-time way of enabling loadinlg 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 diand...@chromium.org
Signed-off-by: Simon Glass s...@chromium.org
---
 README   |9 +
 arch/arm/lib/board.c |   29 +++--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/README b/README
index 22fd6b7..589e22a 100644
--- a/README
+++ b/README
@@ -2311,6 +2311,15 @@ CBFS (Coreboot Filesystem) support
- CONFIG_SYS_VENDOR
- CONFIG_SYS_SOC
 
+   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 2ec6a43..d3053d8 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
@@ -469,7 +470,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
@@ -575,7 +597,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

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot