[10/23] arm: imx: hab: Add IVT header verification

Message ID 1514377566-28512-11-git-send-email-bryan.odonoghue@linaro.org
State New
Headers show
Series
  • Fix and extend i.MX HAB layer
Related show

Commit Message

Bryan O'Donoghue Dec. 27, 2017, 12:25 p.m.
The IVT header contains a magic number, fixed length and one of two version
identifiers. Validate these settings before doing anything with a putative
IVT binary.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
---
 arch/arm/mach-imx/hab.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Comments

Breno Matheus Lima Dec. 28, 2017, 12:59 a.m. | #1
Hi Bryan,

2017-12-27 10:25 GMT-02:00 Bryan O'Donoghue <bryan.odonoghue@linaro.org>:
> The IVT header contains a magic number, fixed length and one of two version
> identifiers. Validate these settings before doing anything with a putative
> IVT binary.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
> Cc: George McCollister <george.mccollister@gmail.com>
> ---
>  arch/arm/mach-imx/hab.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
> index 76267a7..5591cb5 100644
> --- a/arch/arm/mach-imx/hab.c
> +++ b/arch/arm/mach-imx/hab.c
> @@ -229,6 +229,31 @@ uint8_t hab_engines[16] = {
>         -1
>  };
>
> +static int ivt_header_error(const char *err_str, struct ivt_header *ivt_hdr)
> +{
> +       printf("%s magic=0x%x length=0x%02x version=0x%x\n", err_str,
> +              ivt_hdr->magic, ivt_hdr->length, ivt_hdr->version);
> +
> +       return 1;
> +}
> +
> +static int verify_ivt_header(struct ivt_header *ivt_hdr)
> +{
> +       int result = 0;
> +
> +       if (ivt_hdr->magic != IVT_HEADER_MAGIC)
> +               result = ivt_header_error("bad magic", ivt_hdr);
> +
> +       if (be16_to_cpu(ivt_hdr->length) != IVT_TOTAL_LENGTH)
> +               result = ivt_header_error("bad length", ivt_hdr);
> +
> +       if (ivt_hdr->version != IVT_HEADER_V1 &&
> +           ivt_hdr->version != IVT_HEADER_V2)
> +               result = ivt_header_error("bad version", ivt_hdr);
> +
> +       return result;
> +}
> +

I'm trying to build mx6sabreauto which uses the SPL framework and I'm
getting the following build error:

arch/arm/mach-imx/hab.c: In function 'imx_hab_authenticate_image':
arch/arm/mach-imx/hab.c:514:6: warning: implicit declaration of
function 'verify_ivt_header' [-Wimplicit-function-declaration]
  if (verify_ivt_header(ivt_hdr))
      ^
arch/arm/mach-imx/hab.c: At top level:
arch/arm/mach-imx/hab.c:73:13: warning: 'hab_rvt_failsafe_new' defined
but not used [-Wunused-function]
 static void hab_rvt_failsafe_new(void)
             ^
  LD      lib/built-in.o
  LD      spl/arch/arm/mach-imx/built-in.o
  CC      spl/lib/display_options.o
  LD      spl/common/spl/built-in.o
  LD      drivers/video/built-in.o
  LD      drivers/built-in.o
  LD      spl/lib/built-in.o
  LD      u-boot
  LD      spl/u-boot-spl
arch/arm/mach-imx/built-in.o: In function `imx_hab_authenticate_image':
/home/breno/NXP/bootloader/mainline/u-boot-imx/arch/arm/mach-imx/hab.c:514:
undefined reference to `verify_ivt_header'
scripts/Makefile.spl:358: recipe for target 'spl/u-boot-spl' failed
make[1]: *** [spl/u-boot-spl] Error 1
Makefile:1394: recipe for target 'spl/u-boot-spl' failed
make: *** [spl/u-boot-spl] Error 2
make: *** Waiting for unfinished jobs....

Moving the functions ivt_header_error and verify_ivt_header outside of
the "#if !defined(CONFIG_SPL_BUILD)" branch solves this issue in my
side. Can you please check if it's possible to move these functions?

Thanks,
Breno Lima
Bryan O'Donoghue Dec. 28, 2017, 1:35 a.m. | #2
> I'm trying to build mx6sabreauto which uses the SPL framework and I'm
> getting the following build error:
> 
> arch/arm/mach-imx/hab.c: In function 'imx_hab_authenticate_image':
> arch/arm/mach-imx/hab.c:514:6: warning: implicit declaration of
> function 'verify_ivt_header' [-Wimplicit-function-declaration]
>    if (verify_ivt_header(ivt_hdr))
>        ^
> arch/arm/mach-imx/hab.c: At top level:
> arch/arm/mach-imx/hab.c:73:13: warning: 'hab_rvt_failsafe_new' defined
> but not used [-Wunused-function]
>   static void hab_rvt_failsafe_new(void)
>               ^
>    LD      lib/built-in.o
>    LD      spl/arch/arm/mach-imx/built-in.o
>    CC      spl/lib/display_options.o
>    LD      spl/common/spl/built-in.o
>    LD      drivers/video/built-in.o
>    LD      drivers/built-in.o
>    LD      spl/lib/built-in.o
>    LD      u-boot
>    LD      spl/u-boot-spl
> arch/arm/mach-imx/built-in.o: In function `imx_hab_authenticate_image':
> /home/breno/NXP/bootloader/mainline/u-boot-imx/arch/arm/mach-imx/hab.c:514:
> undefined reference to `verify_ivt_header'
> scripts/Makefile.spl:358: recipe for target 'spl/u-boot-spl' failed
> make[1]: *** [spl/u-boot-spl] Error 1
> Makefile:1394: recipe for target 'spl/u-boot-spl' failed
> make: *** [spl/u-boot-spl] Error 2
> make: *** Waiting for unfinished jobs....
> 
> Moving the functions ivt_header_error and verify_ivt_header outside of
> the "#if !defined(CONFIG_SPL_BUILD)" branch solves this issue in my
> side. Can you please check if it's possible to move these functions?

Ah yes I see the problem - thanks I'll fix this straight away.

Patch

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 76267a7..5591cb5 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -229,6 +229,31 @@  uint8_t hab_engines[16] = {
 	-1
 };
 
+static int ivt_header_error(const char *err_str, struct ivt_header *ivt_hdr)
+{
+	printf("%s magic=0x%x length=0x%02x version=0x%x\n", err_str,
+	       ivt_hdr->magic, ivt_hdr->length, ivt_hdr->version);
+
+	return 1;
+}
+
+static int verify_ivt_header(struct ivt_header *ivt_hdr)
+{
+	int result = 0;
+
+	if (ivt_hdr->magic != IVT_HEADER_MAGIC)
+		result = ivt_header_error("bad magic", ivt_hdr);
+
+	if (be16_to_cpu(ivt_hdr->length) != IVT_TOTAL_LENGTH)
+		result = ivt_header_error("bad length", ivt_hdr);
+
+	if (ivt_hdr->version != IVT_HEADER_V1 &&
+	    ivt_hdr->version != IVT_HEADER_V2)
+		result = ivt_header_error("bad version", ivt_hdr);
+
+	return result;
+}
+
 static inline uint8_t get_idx(uint8_t *list, uint8_t tgt)
 {
 	uint8_t idx = 0;
@@ -394,6 +419,8 @@  int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 	hab_rvt_authenticate_image_t *hab_rvt_authenticate_image;
 	hab_rvt_entry_t *hab_rvt_entry;
 	hab_rvt_exit_t *hab_rvt_exit;
+	struct ivt *ivt;
+	struct ivt_header *ivt_hdr;
 
 	hab_rvt_authenticate_image = hab_rvt_authenticate_image_p;
 	hab_rvt_entry = hab_rvt_entry_p;
@@ -416,6 +443,13 @@  int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 
 	/* Calculate IVT address header */
 	ivt_addr = ddr_start + ivt_offset;
+	ivt = (struct ivt *)ivt_addr;
+	ivt_hdr = &ivt->hdr;
+
+	/* Verify IVT header bugging out on error */
+	if (verify_ivt_header(ivt_hdr))
+		goto hab_caam_clock_disable;
+
 	start = ddr_start;
 	bytes = image_size;
 #ifdef DEBUG