[U-Boot] [PATCH 0/4] Allow disabling non-FIT image loading from SPL

[U-Boot] [PATCH 0/4] Allow disabling non-FIT image loading from SPL

[Andrew F. Davis]

Hello all,

To address a needed feature brought up by Andreas[0], we need a way to
disable SPL from loading non-FIT images.

The function spl_parse_image_header is common to all SPL loading paths
(common/spl/spl_(nand|net|nor|etc..)) so we add the check here, much
like the existing CONFIG_SPL_PANIC_ON_RAW_IMAGE.

My original attempt was to add CONFIG_SPL_PANIC_ON_MKIMAGE, but then if other
formats are added, flaws in restricting image types may be introduced, so we
would like a single option to restrict all non-FIT types vs disabling types
individually.

Thanks,
Andrew

[0]: https://www.mail-archive.com/u-boot at lists.denx.de/msg219253.html

Andrew F. Davis (4):
  Introduce CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE
  ARM: AM57xx: Disable non-FIT based image loading for HS devices
  ARM: AM437x: Disable non-FIT based image loading for HS devices
  ARM: DRA7xx: Disable non-FIT based image loading for HS devices

 Kconfig                         | 8 ++++++++
 common/spl/spl.c                | 4 ++++
 configs/am43xx_hs_evm_defconfig | 1 +
 configs/am57xx_hs_evm_defconfig | 1 +
 configs/dra7xx_hs_evm_defconfig | 1 +
 5 files changed, 15 insertions(+)

-- 
2.9.2

[U-Boot] [PATCH 1/4] Introduce CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE

[Andrew F. Davis]

Introduce CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE. An SPL which define
this will panic() if the image it has loaded is not a FIT image.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 Kconfig          | 8 ++++++++
 common/spl/spl.c | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/Kconfig b/Kconfig
index ef12f9f..4c03716 100644
--- a/Kconfig
+++ b/Kconfig
@@ -336,6 +336,14 @@ config SPL_FIT_IMAGE_POST_PROCESS
 	  injected into the FIT creation (i.e. the blobs would have been pre-
 	  processed before being added to the FIT image).
 
+config SPL_PANIC_ON_NON_FIT_IMAGE
+	bool "Disable SPL loading of non-FIT images"
+	help
+	  SPL will panic() if the image loaded is not a FIT image. This is
+	  useful for devices that only support authentication/encryption
+	  through SPL FIT loading paths and do not want SPL falling back
+	  to legacy image loading when a non-FIT image is present.
+
 config SYS_CLK_FREQ
 	depends on ARC || ARCH_SUNXI
 	int "CPU clock frequency"
diff --git a/common/spl/spl.c b/common/spl/spl.c
index b7ec333..7a30c7d 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -93,6 +93,9 @@ void spl_set_header_raw_uboot(void)
 
 int spl_parse_image_header(const struct image_header *header)
 {
+#ifdef CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE
+	panic("** non-FIT images are not supported");
+#else
 	u32 header_size = sizeof(struct image_header);
 
 	if (image_get_magic(header) == IH_MAGIC) {
@@ -153,6 +156,7 @@ int spl_parse_image_header(const struct image_header *header)
 		spl_set_header_raw_uboot();
 #endif
 	}
+#endif
 	return 0;
 }
 
-- 
2.9.2

[U-Boot] [PATCH 2/4] ARM: AM57xx: Disable non-FIT based image loading for HS devices

[Andrew F. Davis]

Disable support for loading non-FIT images for AM57xx platforms using
the high-security (HS) device variant.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 configs/am57xx_hs_evm_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/am57xx_hs_evm_defconfig b/configs/am57xx_hs_evm_defconfig
index a4bfdd5..e6f3ebc 100644
--- a/configs/am57xx_hs_evm_defconfig
+++ b/configs/am57xx_hs_evm_defconfig
@@ -41,6 +41,7 @@ CONFIG_FIT=y
 CONFIG_SPL_OF_LIBFDT=y
 CONFIG_SPL_LOAD_FIT=y
 CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y
+CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE=y
 CONFIG_OF_LIST="am57xx-beagle-x15"
 CONFIG_DM_I2C=y
 CONFIG_DM_SPI=y
-- 
2.9.2

[U-Boot] [PATCH 3/4] ARM: AM437x: Disable non-FIT based image loading for HS devices

[Andrew F. Davis]

Disable support for loading non-FIT images for AM437x platforms using
the high-security (HS) device variant.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 configs/am43xx_hs_evm_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/am43xx_hs_evm_defconfig b/configs/am43xx_hs_evm_defconfig
index c8ce723..fc2b8b0 100644
--- a/configs/am43xx_hs_evm_defconfig
+++ b/configs/am43xx_hs_evm_defconfig
@@ -14,6 +14,7 @@ CONFIG_FIT=y
 CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=1, NAND"
 CONFIG_SPL_LOAD_FIT=y
 CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y
+CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE=y
 CONFIG_HUSH_PARSER=y
 CONFIG_CMD_BOOTZ=y
 # CONFIG_CMD_IMLS is not set
-- 
2.9.2

[U-Boot] [PATCH 4/4] ARM: DRA7xx: Disable non-FIT based image loading for HS devices

[Andrew F. Davis]

Disable support for loading non-FIT images for DRA7xx platforms using
the high-security (HS) device variant.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 configs/dra7xx_hs_evm_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/dra7xx_hs_evm_defconfig b/configs/dra7xx_hs_evm_defconfig
index faf9cd5..63801fe 100644
--- a/configs/dra7xx_hs_evm_defconfig
+++ b/configs/dra7xx_hs_evm_defconfig
@@ -59,5 +59,6 @@ CONFIG_FIT=y
 CONFIG_SPL_OF_LIBFDT=y
 CONFIG_SPL_LOAD_FIT=y
 CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y
+CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE=y
 CONFIG_OF_LIST="dra7-evm dra72-evm"
 CONFIG_DM_I2C=y
-- 
2.9.2

[U-Boot] [PATCH 1/4] Introduce CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE

[Tom Rini]

On Mon, Aug 01, 2016 at 10:30:23AM -0500, Andrew F. Davis wrote:
> Introduce CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE. An SPL which define
> this will panic() if the image it has loaded is not a FIT image.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---
>  Kconfig          | 8 ++++++++
>  common/spl/spl.c | 4 ++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/Kconfig b/Kconfig
> index ef12f9f..4c03716 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -336,6 +336,14 @@ config SPL_FIT_IMAGE_POST_PROCESS
>  	  injected into the FIT creation (i.e. the blobs would have been pre-
>  	  processed before being added to the FIT image).
>  
> +config SPL_PANIC_ON_NON_FIT_IMAGE
> +	bool "Disable SPL loading of non-FIT images"

I think we should make this default y if SPL_FIT_SIGNATURE since the
point of enabling these options is that you want to be verified or
failing.  During development you can disable this easily enough.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160801/4998d2da/attachment.sig>

[U-Boot] [PATCH 1/4] Introduce CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE

[Simon Glass]

Hi,

On 1 August 2016 at 11:30, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Aug 01, 2016 at 10:30:23AM -0500, Andrew F. Davis wrote:
> > Introduce CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE. An SPL which define
> > this will panic() if the image it has loaded is not a FIT image.
> >
> > Signed-off-by: Andrew F. Davis <afd@ti.com>
> > ---
> >  Kconfig          | 8 ++++++++
> >  common/spl/spl.c | 4 ++++
> >  2 files changed, 12 insertions(+)
> >
> > diff --git a/Kconfig b/Kconfig
> > index ef12f9f..4c03716 100644
> > --- a/Kconfig
> > +++ b/Kconfig
> > @@ -336,6 +336,14 @@ config SPL_FIT_IMAGE_POST_PROCESS
> >         injected into the FIT creation (i.e. the blobs would have been pre-
> >         processed before being added to the FIT image).
> >
> > +config SPL_PANIC_ON_NON_FIT_IMAGE
> > +     bool "Disable SPL loading of non-FIT images"
>
> I think we should make this default y if SPL_FIT_SIGNATURE since the
> point of enabling these options is that you want to be verified or
> failing.  During development you can disable this easily enough.

Also can we reuse CONFIG_IMAGE_FORMAT_LEGACY?

Regards,
Simon

[U-Boot] [PATCH 2/4] ARM: AM57xx: Disable non-FIT based image loading for HS devices

[Simon Glass]

Hi Andrew,

On 1 August 2016 at 09:30, Andrew F. Davis <afd@ti.com> wrote:
> Disable support for loading non-FIT images for AM57xx platforms using
> the high-security (HS) device variant.
>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---
>  configs/am57xx_hs_evm_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configs/am57xx_hs_evm_defconfig b/configs/am57xx_hs_evm_defconfig
> index a4bfdd5..e6f3ebc 100644
> --- a/configs/am57xx_hs_evm_defconfig
> +++ b/configs/am57xx_hs_evm_defconfig
> @@ -41,6 +41,7 @@ CONFIG_FIT=y
>  CONFIG_SPL_OF_LIBFDT=y
>  CONFIG_SPL_LOAD_FIT=y
>  CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y
> +CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE=y
>  CONFIG_OF_LIST="am57xx-beagle-x15"
>  CONFIG_DM_I2C=y
>  CONFIG_DM_SPI=y
> --
> 2.9.2
>

Why do we need to panic? Can we use something like CONFIG_IMAGE_FORMAT_LEGACY?

Regards,
Simon

[U-Boot] [PATCH 2/4] ARM: AM57xx: Disable non-FIT based image loading for HS devices

[Tom Rini]

On Wed, Aug 03, 2016 at 07:16:12PM -0600, Simon Glass wrote:
> Hi Andrew,
> 
> On 1 August 2016 at 09:30, Andrew F. Davis <afd@ti.com> wrote:
> > Disable support for loading non-FIT images for AM57xx platforms using
> > the high-security (HS) device variant.
> >
> > Signed-off-by: Andrew F. Davis <afd@ti.com>
> > ---
> >  configs/am57xx_hs_evm_defconfig | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/configs/am57xx_hs_evm_defconfig b/configs/am57xx_hs_evm_defconfig
> > index a4bfdd5..e6f3ebc 100644
> > --- a/configs/am57xx_hs_evm_defconfig
> > +++ b/configs/am57xx_hs_evm_defconfig
> > @@ -41,6 +41,7 @@ CONFIG_FIT=y
> >  CONFIG_SPL_OF_LIBFDT=y
> >  CONFIG_SPL_LOAD_FIT=y
> >  CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y
> > +CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE=y
> >  CONFIG_OF_LIST="am57xx-beagle-x15"
> >  CONFIG_DM_I2C=y
> >  CONFIG_DM_SPI=y
> 
> Why do we need to panic? Can we use something like CONFIG_IMAGE_FORMAT_LEGACY?

The need to panic is because if we cannot authenticate we are to fail as
this is a secure device.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160803/aaaee169/attachment.sig>

[U-Boot] [PATCH 2/4] ARM: AM57xx: Disable non-FIT based image loading for HS devices

[Simon Glass]

Hi Tom,

On 3 August 2016 at 19:48, Tom Rini <trini@konsulko.com> wrote:
> On Wed, Aug 03, 2016 at 07:16:12PM -0600, Simon Glass wrote:
>> Hi Andrew,
>>
>> On 1 August 2016 at 09:30, Andrew F. Davis <afd@ti.com> wrote:
>> > Disable support for loading non-FIT images for AM57xx platforms using
>> > the high-security (HS) device variant.
>> >
>> > Signed-off-by: Andrew F. Davis <afd@ti.com>
>> > ---
>> >  configs/am57xx_hs_evm_defconfig | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/configs/am57xx_hs_evm_defconfig b/configs/am57xx_hs_evm_defconfig
>> > index a4bfdd5..e6f3ebc 100644
>> > --- a/configs/am57xx_hs_evm_defconfig
>> > +++ b/configs/am57xx_hs_evm_defconfig
>> > @@ -41,6 +41,7 @@ CONFIG_FIT=y
>> >  CONFIG_SPL_OF_LIBFDT=y
>> >  CONFIG_SPL_LOAD_FIT=y
>> >  CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y
>> > +CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE=y
>> >  CONFIG_OF_LIST="am57xx-beagle-x15"
>> >  CONFIG_DM_I2C=y
>> >  CONFIG_DM_SPI=y
>>
>> Why do we need to panic? Can we use something like CONFIG_IMAGE_FORMAT_LEGACY?
>
> The need to panic is because if we cannot authenticate we are to fail as
> this is a secure device.

We already have a hang in board_init_r() (SPL: failed to boot from all
boot devices). I'm suggesting that instead of panic we just return an
error from spl_parse_image_header() and let things work in the normal
way.

Anyway, I don't mind if people prefer this approach.

It's strange that this patch does not seem to be in patchwork.

Regards,
Simon

[U-Boot] [PATCH 2/4] ARM: AM57xx: Disable non-FIT based image loading for HS devices

[Andrew F. Davis]

On 08/03/2016 09:27 PM, Simon Glass wrote:
> Hi Tom,
> 
> On 3 August 2016 at 19:48, Tom Rini <trini@konsulko.com> wrote:
>> On Wed, Aug 03, 2016 at 07:16:12PM -0600, Simon Glass wrote:
>>> Hi Andrew,
>>>
>>> On 1 August 2016 at 09:30, Andrew F. Davis <afd@ti.com> wrote:
>>>> Disable support for loading non-FIT images for AM57xx platforms using
>>>> the high-security (HS) device variant.
>>>>
>>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>>>> ---
>>>>  configs/am57xx_hs_evm_defconfig | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/configs/am57xx_hs_evm_defconfig b/configs/am57xx_hs_evm_defconfig
>>>> index a4bfdd5..e6f3ebc 100644
>>>> --- a/configs/am57xx_hs_evm_defconfig
>>>> +++ b/configs/am57xx_hs_evm_defconfig
>>>> @@ -41,6 +41,7 @@ CONFIG_FIT=y
>>>>  CONFIG_SPL_OF_LIBFDT=y
>>>>  CONFIG_SPL_LOAD_FIT=y
>>>>  CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y
>>>> +CONFIG_SPL_PANIC_ON_NON_FIT_IMAGE=y
>>>>  CONFIG_OF_LIST="am57xx-beagle-x15"
>>>>  CONFIG_DM_I2C=y
>>>>  CONFIG_DM_SPI=y
>>>
>>> Why do we need to panic? Can we use something like CONFIG_IMAGE_FORMAT_LEGACY?
>>
>> The need to panic is because if we cannot authenticate we are to fail as
>> this is a secure device.
> 
> We already have a hang in board_init_r() (SPL: failed to boot from all
> boot devices). I'm suggesting that instead of panic we just return an
> error from spl_parse_image_header() and let things work in the normal
> way.
> 

I think this will work also, it looks like CONFIG_IMAGE_FORMAT_LEGACY
was made for a similar reason as we have here just for the non-SPL
u-boot case.

> Anyway, I don't mind if people prefer this approach.
> 

I'll investigate this and see if it can be made into an SPL equivalent
version.

Thanks,
Andrew

> It's strange that this patch does not seem to be in patchwork.
> 
> Regards,
> Simon
>