[U-Boot] [PATCH 1/2] usbarmory: switch to using kernel zImage

[U-Boot] [PATCH 1/2] usbarmory: switch to using kernel zImage

[andrej at inversepath.com]

From: Andrej Rosano <andrej@inversepath.com>

Switch to using zImage instead of uImage.

Signed-off-by: Andrej Rosano <andrej@inversepath.com>
---
 include/configs/usbarmory.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/configs/usbarmory.h b/include/configs/usbarmory.h
index 8568663..c0e093f 100644
--- a/include/configs/usbarmory.h
+++ b/include/configs/usbarmory.h
@@ -69,17 +69,19 @@
 #define CONFIG_CMD_FUSE
 #define CONFIG_FSL_IIM
 
-/* Linux boot */
+/* U-Boot memory offsets */
 #define CONFIG_LOADADDR		0x72000000
 #define CONFIG_SYS_TEXT_BASE	0x77800000
 #define CONFIG_SYS_LOAD_ADDR	CONFIG_LOADADDR
+
+/* Linux boot */
 #define CONFIG_HOSTNAME		usbarmory
 #define CONFIG_BOOTCOMMAND						\
 	"run distro_bootcmd; "						\
 	"setenv bootargs console=${console} ${bootargs_default}; "	\
-	"ext2load mmc 0:1 ${kernel_addr_r} /boot/uImage; "		\
+	"ext2load mmc 0:1 ${kernel_addr_r} /boot/zImage; "		\
 	"ext2load mmc 0:1 ${fdt_addr_r} /boot/${fdtfile}; "		\
-	"bootm ${kernel_addr_r} - ${fdt_addr_r}"
+	"bootz ${kernel_addr_r} - ${fdt_addr_r}"
 
 #define BOOT_TARGET_DEVICES(func) func(MMC, mmc, 0)
 
-- 
2.9.0

[U-Boot] [PATCH 2/2] usbarmory: Add board_run_command() function

[andrej at inversepath.com]

From: Andrej Rosano <andrej@inversepath.com>

Define a default board_run_command() function. This function contains
the commands needed to boot the board when CLI is disabled (CONFIG_CMDLINE=n).

Signed-off-by: Andrej Rosano <andrej@inversepath.com>
---
 board/inversepath/usbarmory/usbarmory.c | 31 +++++++++++++++++++++++++++++++
 include/configs/usbarmory.h             | 11 +++++++----
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/board/inversepath/usbarmory/usbarmory.c b/board/inversepath/usbarmory/usbarmory.c
index a809039..a11e3b3 100644
--- a/board/inversepath/usbarmory/usbarmory.c
+++ b/board/inversepath/usbarmory/usbarmory.c
@@ -415,3 +415,34 @@ int checkboard(void)
 	puts("Board: Inverse Path USB armory MkI\n");
 	return 0;
 }
+
+#ifndef CONFIG_CMDLINE
+static char *ext2_argv[] = {
+	"ext2load",
+	"mmc",
+	"0:1",
+	USBARMORY_FIT_ADDR,
+	USBARMORY_FIT_PATH
+};
+
+static char *bootm_argv[] = {
+	"bootm",
+	USBARMORY_FIT_ADDR
+};
+
+int board_run_command(const char *cmdline)
+{
+	printf("%s %s %s %s %s\n", ext2_argv[0], ext2_argv[1], ext2_argv[2],
+	       ext2_argv[3], ext2_argv[4]);
+
+	if (do_ext2load(NULL, 0, 5, ext2_argv) != 0) {
+		udelay(5*1000*1000);
+		return 1;
+	}
+
+	printf("%s %s\n", bootm_argv[0], bootm_argv[1]);
+	do_bootm(NULL, 0, 2, bootm_argv);
+
+	return 1;
+}
+#endif
diff --git a/include/configs/usbarmory.h b/include/configs/usbarmory.h
index c0e093f..5484204 100644
--- a/include/configs/usbarmory.h
+++ b/include/configs/usbarmory.h
@@ -17,16 +17,13 @@
 #define CONFIG_SYS_FSL_CLK
 #define CONFIG_BOARD_EARLY_INIT_F
 #define CONFIG_MXC_GPIO
+#define CONFIG_SYS_NO_FLASH
 
 #include <asm/arch/imx-regs.h>
 
 #include <config_distro_defaults.h>
 
-/* U-Boot commands */
-
 /* U-Boot environment */
-#define CONFIG_ENV_OVERWRITE
-#define CONFIG_SYS_NO_FLASH
 #define CONFIG_ENV_OFFSET	(6 * 64 * 1024)
 #define CONFIG_ENV_SIZE		(8 * 1024)
 #define CONFIG_ENV_IS_IN_MMC
@@ -101,6 +98,12 @@
 	"console=ttymxc0,115200\0"				\
 	BOOTENV
 
+#ifndef CONFIG_CMDLINE
+#define CONFIG_BOOTARGS "console=ttymxc0,115200 root=/dev/mmcblk0p1 rootwait rw"
+#define USBARMORY_FIT_PATH	"/boot/usbarmory.itb"
+#define USBARMORY_FIT_ADDR	"0x70800000"
+#endif
+
 /* Physical Memory Map */
 #define CONFIG_NR_DRAM_BANKS		1
 #define PHYS_SDRAM			CSD0_BASE_ADDR
-- 
2.9.0

[U-Boot] [PATCH 2/2] usbarmory: Add board_run_command() function

[Stefano Babic]

Hallo Andrej,

On 20/06/2016 17:21, andrej at inversepath.com wrote:
> From: Andrej Rosano <andrej@inversepath.com>
> 
> Define a default board_run_command() function. This function contains
> the commands needed to boot the board when CLI is disabled (CONFIG_CMDLINE=n).
> 
> Signed-off-by: Andrej Rosano <andrej@inversepath.com>
> ---
>  board/inversepath/usbarmory/usbarmory.c | 31 +++++++++++++++++++++++++++++++
>  include/configs/usbarmory.h             | 11 +++++++----
>  2 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/board/inversepath/usbarmory/usbarmory.c b/board/inversepath/usbarmory/usbarmory.c
> index a809039..a11e3b3 100644
> --- a/board/inversepath/usbarmory/usbarmory.c
> +++ b/board/inversepath/usbarmory/usbarmory.c
> @@ -415,3 +415,34 @@ int checkboard(void)
>  	puts("Board: Inverse Path USB armory MkI\n");
>  	return 0;
>  }
> +
> +#ifndef CONFIG_CMDLINE
> +static char *ext2_argv[] = {
> +	"ext2load",
> +	"mmc",
> +	"0:1",
> +	USBARMORY_FIT_ADDR,
> +	USBARMORY_FIT_PATH
> +};
> +
> +static char *bootm_argv[] = {
> +	"bootm",
> +	USBARMORY_FIT_ADDR
> +};
> +
> +int board_run_command(const char *cmdline)
> +{
> +	printf("%s %s %s %s %s\n", ext2_argv[0], ext2_argv[1], ext2_argv[2],
> +	       ext2_argv[3], ext2_argv[4]);
> +
> +	if (do_ext2load(NULL, 0, 5, ext2_argv) != 0) {
> +		udelay(5*1000*1000);
> +		return 1;
> +	}
> +
> +	printf("%s %s\n", bootm_argv[0], bootm_argv[1]);
> +	do_bootm(NULL, 0, 2, bootm_argv);
> +
> +	return 1;
> +}

I ten to NACK this. You can do exactly the same with a U-Boot script,
and if you want to have this as default, you can change your default
environment. This is just a wrapper around the hush shell.


> +#endif
> diff --git a/include/configs/usbarmory.h b/include/configs/usbarmory.h
> index c0e093f..5484204 100644
> --- a/include/configs/usbarmory.h
> +++ b/include/configs/usbarmory.h
> @@ -17,16 +17,13 @@
>  #define CONFIG_SYS_FSL_CLK
>  #define CONFIG_BOARD_EARLY_INIT_F
>  #define CONFIG_MXC_GPIO
> +#define CONFIG_SYS_NO_FLASH
>  
>  #include <asm/arch/imx-regs.h>
>  
>  #include <config_distro_defaults.h>
>  
> -/* U-Boot commands */
> -
>  /* U-Boot environment */
> -#define CONFIG_ENV_OVERWRITE
> -#define CONFIG_SYS_NO_FLASH
>  #define CONFIG_ENV_OFFSET	(6 * 64 * 1024)
>  #define CONFIG_ENV_SIZE		(8 * 1024)
>  #define CONFIG_ENV_IS_IN_MMC
> @@ -101,6 +98,12 @@
>  	"console=ttymxc0,115200\0"				\
>  	BOOTENV
>  
> +#ifndef CONFIG_CMDLINE
> +#define CONFIG_BOOTARGS "console=ttymxc0,115200 root=/dev/mmcblk0p1 rootwait rw"
> +#define USBARMORY_FIT_PATH	"/boot/usbarmory.itb"
> +#define USBARMORY_FIT_ADDR	"0x70800000"
> +#endif
> +
>  /* Physical Memory Map */
>  #define CONFIG_NR_DRAM_BANKS		1
>  #define PHYS_SDRAM			CSD0_BASE_ADDR
> 

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 2/2] usbarmory: Add board_run_command() function

[Andrej Rosano]

Hi Stefano,

On Mon, Jun 20, 2016 at 05:46:52PM +0200, Stefano Babic wrote:
> Hallo Andrej,
> 
> On 20/06/2016 17:21, andrej at inversepath.com wrote:
> > From: Andrej Rosano <andrej@inversepath.com>
> > 
> > Define a default board_run_command() function. This function contains
> > the commands needed to boot the board when CLI is disabled (CONFIG_CMDLINE=n).
> > 
> > Signed-off-by: Andrej Rosano <andrej@inversepath.com>
> > ---
> >  board/inversepath/usbarmory/usbarmory.c | 31 +++++++++++++++++++++++++++++++
> >  include/configs/usbarmory.h             | 11 +++++++----
> >  2 files changed, 38 insertions(+), 4 deletions(-)
> > 
> > diff --git a/board/inversepath/usbarmory/usbarmory.c b/board/inversepath/usbarmory/usbarmory.c
> > index a809039..a11e3b3 100644
> > --- a/board/inversepath/usbarmory/usbarmory.c
> > +++ b/board/inversepath/usbarmory/usbarmory.c
> > @@ -415,3 +415,34 @@ int checkboard(void)
> >  	puts("Board: Inverse Path USB armory MkI\n");
> >  	return 0;
> >  }
> > +
> > +#ifndef CONFIG_CMDLINE
> > +static char *ext2_argv[] = {
> > +	"ext2load",
> > +	"mmc",
> > +	"0:1",
> > +	USBARMORY_FIT_ADDR,
> > +	USBARMORY_FIT_PATH
> > +};
> > +
> > +static char *bootm_argv[] = {
> > +	"bootm",
> > +	USBARMORY_FIT_ADDR
> > +};
> > +
> > +int board_run_command(const char *cmdline)
> > +{
> > +	printf("%s %s %s %s %s\n", ext2_argv[0], ext2_argv[1], ext2_argv[2],
> > +	       ext2_argv[3], ext2_argv[4]);
> > +
> > +	if (do_ext2load(NULL, 0, 5, ext2_argv) != 0) {
> > +		udelay(5*1000*1000);
> > +		return 1;
> > +	}
> > +
> > +	printf("%s %s\n", bootm_argv[0], bootm_argv[1]);
> > +	do_bootm(NULL, 0, 2, bootm_argv);
> > +
> > +	return 1;
> > +}
> 
> I ten to NACK this. You can do exactly the same with a U-Boot script,
> and if you want to have this as default, you can change your default
> environment. This is just a wrapper around the hush shell.

The intention of the patch is to boot the kernel while having the CLI disabled
(CONFIG_CMDLINE=n). The U-Boot script needs the CLI to be enabled AFAIK.

It is better having the CLI disabled when using the Verified Boot, otherwise
there are chances to bypass the FIT image verification (e.g. using md/mw
commands in case are available):

https://github.com/inversepath/usbarmory/blob/master/software/secure_boot/u-boot-2016.05_patches/0003-Disable-CLI.patch

Andrej

> 
> 
> > +#endif
> > diff --git a/include/configs/usbarmory.h b/include/configs/usbarmory.h
> > index c0e093f..5484204 100644
> > --- a/include/configs/usbarmory.h
> > +++ b/include/configs/usbarmory.h
> > @@ -17,16 +17,13 @@
> >  #define CONFIG_SYS_FSL_CLK
> >  #define CONFIG_BOARD_EARLY_INIT_F
> >  #define CONFIG_MXC_GPIO
> > +#define CONFIG_SYS_NO_FLASH
> >  
> >  #include <asm/arch/imx-regs.h>
> >  
> >  #include <config_distro_defaults.h>
> >  
> > -/* U-Boot commands */
> > -
> >  /* U-Boot environment */
> > -#define CONFIG_ENV_OVERWRITE
> > -#define CONFIG_SYS_NO_FLASH
> >  #define CONFIG_ENV_OFFSET	(6 * 64 * 1024)
> >  #define CONFIG_ENV_SIZE		(8 * 1024)
> >  #define CONFIG_ENV_IS_IN_MMC
> > @@ -101,6 +98,12 @@
> >  	"console=ttymxc0,115200\0"				\
> >  	BOOTENV
> >  
> > +#ifndef CONFIG_CMDLINE
> > +#define CONFIG_BOOTARGS "console=ttymxc0,115200 root=/dev/mmcblk0p1 rootwait rw"
> > +#define USBARMORY_FIT_PATH	"/boot/usbarmory.itb"
> > +#define USBARMORY_FIT_ADDR	"0x70800000"
> > +#endif
> > +
> >  /* Physical Memory Map */
> >  #define CONFIG_NR_DRAM_BANKS		1
> >  #define PHYS_SDRAM			CSD0_BASE_ADDR
> > 
> 
> Best regards,
> Stefano Babic
> 
> -- 
> =====================================================================
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> =====================================================================

--
Andrej Rosano                                           Inverse Path Srl
<andrej@inversepath.com>                      http://www.inversepath.com

0x01939B21            5BB8 574E 68E8 D841 E18F  D5E9 CEAD E0CF 0193 9B21

[U-Boot] [PATCH 2/2] usbarmory: Add board_run_command() function

[Stefano Babic]

Hi Andrej,

On 20/06/2016 18:18, Andrej Rosano wrote:

>>
>> I ten to NACK this. You can do exactly the same with a U-Boot script,
>> and if you want to have this as default, you can change your default
>> environment. This is just a wrapper around the hush shell.
> 
> The intention of the patch is to boot the kernel while having the CLI disabled
> (CONFIG_CMDLINE=n). The U-Boot script needs the CLI to be enabled AFAIK.
> 
> It is better having the CLI disabled when using the Verified Boot, otherwise
> there are chances to bypass the FIT image verification (e.g. using md/mw
> commands in case are available):

Why is it not enough to disable the CONSOLE ? I mean, if there is no
user interface (and this is done in a lot of ways, for example setting
stdin / stdout), there is no ways to bypass it because the interface is
not availabel. Or is there some other security issues I am not aware of ?

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 2/2] usbarmory: Add board_run_command() function

[Simon Glass]

Hi,

On 27 June 2016 at 03:38, Stefano Babic <sbabic@denx.de> wrote:
> Hi Andrej,
>
> On 20/06/2016 18:18, Andrej Rosano wrote:
>
>>>
>>> I ten to NACK this. You can do exactly the same with a U-Boot script,
>>> and if you want to have this as default, you can change your default
>>> environment. This is just a wrapper around the hush shell.
>>
>> The intention of the patch is to boot the kernel while having the CLI disabled
>> (CONFIG_CMDLINE=n). The U-Boot script needs the CLI to be enabled AFAIK.
>>
>> It is better having the CLI disabled when using the Verified Boot, otherwise
>> there are chances to bypass the FIT image verification (e.g. using md/mw
>> commands in case are available):
>
> Why is it not enough to disable the CONSOLE ? I mean, if there is no
> user interface (and this is done in a lot of ways, for example setting
> stdin / stdout), there is no ways to bypass it because the interface is
> not availabel. Or is there some other security issues I am not aware of ?

It is an extra level of security - providing a very simple command
execution instead of the general CLI. That is actually the original
purpose of board_run_command(). E.g. for Chrome OS we had an option to
either run the normal CLI or a simple (secure) one. Also see
cli_process_fdt() which provides for a 'bootsecure' mode, controlled
from the FDT.

Regards,
Simon

[U-Boot] [PATCH 2/2] usbarmory: Add board_run_command() function

[Stefano Babic]

Hi Simon,

On 28/06/2016 20:43, Simon Glass wrote:
> Hi,
> 
> On 27 June 2016 at 03:38, Stefano Babic <sbabic@denx.de> wrote:
>> Hi Andrej,
>>
>> On 20/06/2016 18:18, Andrej Rosano wrote:
>>
>>>>
>>>> I ten to NACK this. You can do exactly the same with a U-Boot script,
>>>> and if you want to have this as default, you can change your default
>>>> environment. This is just a wrapper around the hush shell.
>>>
>>> The intention of the patch is to boot the kernel while having the CLI disabled
>>> (CONFIG_CMDLINE=n). The U-Boot script needs the CLI to be enabled AFAIK.
>>>
>>> It is better having the CLI disabled when using the Verified Boot, otherwise
>>> there are chances to bypass the FIT image verification (e.g. using md/mw
>>> commands in case are available):
>>
>> Why is it not enough to disable the CONSOLE ? I mean, if there is no
>> user interface (and this is done in a lot of ways, for example setting
>> stdin / stdout), there is no ways to bypass it because the interface is
>> not availabel. Or is there some other security issues I am not aware of ?
> 
> It is an extra level of security - providing a very simple command
> execution instead of the general CLI. That is actually the original
> purpose of board_run_command(). E.g. for Chrome OS we had an option to
> either run the normal CLI or a simple (secure) one. Also see
> cli_process_fdt() which provides for a 'bootsecure' mode, controlled
> from the FDT.

I see, thanks for explanation. My fear is that the process diverges and
boards start to embed U-Boot scripts inside the code, letting them not
very maintainable. But I have understood the issue and I put this patch
for merging in my queue.

Best regards,
Stefano


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================