| Message ID | 1514377566-28512-11-git-send-email-bryan.odonoghue@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix and extend i.MX HAB layer | expand |
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
> 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.
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
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(+)