Both usages of authenticate_image treat the result code as a simple binary. The command line usage of authenticate_image directly returns the result code of authenticate_image as a success/failure code.

Right now when calling hab_auth_img and test the result code in a shell a passing hab_auth_img will appear to the shell as a fail.

The first step in fixing this behaviour is to fix-up the result code return by authenticate_image() itself, subsequent patches fix the interpretation of authenticate_image so that zero will return CMD_RET_SUCCESS and non-zero will return CMD_RET_FAILURE.

The first step is fixing the return type in authenticate_image() so do that now.

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/include/asm/mach-imx/hab.h | 2 +- arch/arm/mach-imx/hab.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h index e0ff459..1b7a5e4 100644 --- a/arch/arm/include/asm/mach-imx/hab.h +++ b/arch/arm/include/asm/mach-imx/hab.h @@ -145,6 +145,6 @@ typedef void hapi_clock_init_t(void);

/* ----------- end of HAB API updates ------------*/

-uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size); +int authenticate_image(uint32_t ddr_start, uint32_t image_size);

#endif diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 02c7ae4..09892a6 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -410,7 +410,7 @@ static bool is_hab_enabled(void) return (reg & IS_HAB_ENABLED_BIT) == IS_HAB_ENABLED_BIT; }

-uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size) +int authenticate_image(uint32_t ddr_start, uint32_t image_size) { uint32_t load_addr = 0; size_t bytes;

There is no need to call is_enabled() twice in authenticate_image - it does nothing but add an additional layer of indentation.

We can check for is_enabled() at the start of the function and return the result code directly.

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/mach-imx/hab.c | 138 ++++++++++++++++++++++++------------------------ 1 file changed, 69 insertions(+), 69 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 9fe6d43..6f86c02 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -428,91 +428,91 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size) hab_rvt_entry = hab_rvt_entry_p; hab_rvt_exit = hab_rvt_exit_p;

- if (is_hab_enabled()) { - printf("\nAuthenticate image from DDR location 0x%x...\n", - ddr_start); + if (!is_hab_enabled()) { + puts("hab fuse not enabled\n"); + return result; + }

- hab_caam_clock_enable(1); + printf("\nAuthenticate image from DDR location 0x%x...\n", + ddr_start);

- if (hab_rvt_entry() == HAB_SUCCESS) { - /* If not already aligned, Align to ALIGN_SIZE */ - ivt_offset = (image_size + ALIGN_SIZE - 1) & - ~(ALIGN_SIZE - 1); + hab_caam_clock_enable(1);

- start = ddr_start; - bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE; + if (hab_rvt_entry() == HAB_SUCCESS) { + /* If not already aligned, Align to ALIGN_SIZE */ + ivt_offset = (image_size + ALIGN_SIZE - 1) & + ~(ALIGN_SIZE - 1); + + start = ddr_start; + bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE; #ifdef DEBUG - printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", - ivt_offset, ddr_start + ivt_offset); - puts("Dumping IVT\n"); - print_buffer(ddr_start + ivt_offset, - (void *)(ddr_start + ivt_offset), - 4, 0x8, 0); - - puts("Dumping CSF Header\n"); - print_buffer(ddr_start + ivt_offset+IVT_SIZE, - (void *)(ddr_start + ivt_offset+IVT_SIZE), - 4, 0x10, 0); + printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", + ivt_offset, ddr_start + ivt_offset); + puts("Dumping IVT\n"); + print_buffer(ddr_start + ivt_offset, + (void *)(ddr_start + ivt_offset), + 4, 0x8, 0); + + puts("Dumping CSF Header\n"); + print_buffer(ddr_start + ivt_offset + IVT_SIZE, + (void *)(ddr_start + ivt_offset + IVT_SIZE), + 4, 0x10, 0);

#if !defined(CONFIG_SPL_BUILD) - get_hab_status(); + get_hab_status(); #endif

- puts("\nCalling authenticate_image in ROM\n"); - printf("\tivt_offset = 0x%x\n", ivt_offset); - printf("\tstart = 0x%08lx\n", start); - printf("\tbytes = 0x%x\n", bytes); + puts("\nCalling authenticate_image in ROM\n"); + printf("\tivt_offset = 0x%x\n", ivt_offset); + printf("\tstart = 0x%08lx\n", start); + printf("\tbytes = 0x%x\n", bytes); #endif - /* - * If the MMU is enabled, we have to notify the ROM - * code, or it won't flush the caches when needed. - * This is done, by setting the "pu_irom_mmu_enabled" - * word to 1. You can find its address by looking in - * the ROM map. This is critical for - * authenticate_image(). If MMU is enabled, without - * setting this bit, authentication will fail and may - * crash. - */ - /* Check MMU enabled */ - if (is_soc_type(MXC_SOC_MX6) && get_cr() & CR_M) { - if (is_mx6dq()) { - /* - * This won't work on Rev 1.0.0 of - * i.MX6Q/D, since their ROM doesn't - * do cache flushes. don't think any - * exist, so we ignore them. - */ - if (!is_mx6dqp()) - writel(1, MX6DQ_PU_IROM_MMU_EN_VAR); - } else if (is_mx6sdl()) { - writel(1, MX6DLS_PU_IROM_MMU_EN_VAR); - } else if (is_mx6sl()) { - writel(1, MX6SL_PU_IROM_MMU_EN_VAR); - } + /* + * If the MMU is enabled, we have to notify the ROM + * code, or it won't flush the caches when needed. + * This is done, by setting the "pu_irom_mmu_enabled" + * word to 1. You can find its address by looking in + * the ROM map. This is critical for + * authenticate_image(). If MMU is enabled, without + * setting this bit, authentication will fail and may + * crash. + */ + /* Check MMU enabled */ + if (is_soc_type(MXC_SOC_MX6) && get_cr() & CR_M) { + if (is_mx6dq()) { + /* + * This won't work on Rev 1.0.0 of + * i.MX6Q/D, since their ROM doesn't + * do cache flushes. don't think any + * exist, so we ignore them. + */ + if (!is_mx6dqp()) + writel(1, MX6DQ_PU_IROM_MMU_EN_VAR); + } else if (is_mx6sdl()) { + writel(1, MX6DLS_PU_IROM_MMU_EN_VAR); + } else if (is_mx6sl()) { + writel(1, MX6SL_PU_IROM_MMU_EN_VAR); } + }

- load_addr = (uint32_t)hab_rvt_authenticate_image( - HAB_CID_UBOOT, - ivt_offset, (void **)&start, - (size_t *)&bytes, NULL); - if (hab_rvt_exit() != HAB_SUCCESS) { - puts("hab exit function fail\n"); - load_addr = 0; - } - } else { - puts("hab entry function fail\n"); + load_addr = (uint32_t)hab_rvt_authenticate_image( + HAB_CID_UBOOT, + ivt_offset, (void **)&start, + (size_t *)&bytes, NULL); + if (hab_rvt_exit() != HAB_SUCCESS) { + puts("hab exit function fail\n"); + load_addr = 0; } + } else { + puts("hab entry function fail\n"); + }

- hab_caam_clock_enable(0); + hab_caam_clock_enable(0);

#if !defined(CONFIG_SPL_BUILD) - get_hab_status(); + get_hab_status(); #endif - } else { - puts("hab fuse not enabled\n"); - } - - if ((!is_hab_enabled()) || (load_addr != 0)) + if (load_addr != 0) result = 0;

return result;

The size of the IVT header should be defined in hab.h move it there now.

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/include/asm/mach-imx/hab.h | 2 ++ arch/arm/mach-imx/hab.c | 1 - 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h index 1b7a5e4..3c19d2e 100644 --- a/arch/arm/include/asm/mach-imx/hab.h +++ b/arch/arm/include/asm/mach-imx/hab.h @@ -143,6 +143,8 @@ typedef void hapi_clock_init_t(void); #define HAB_CID_ROM 0 /**< ROM Caller ID */ #define HAB_CID_UBOOT 1 /**< UBOOT Caller ID*/

+#define IVT_SIZE 0x20 + /* ----------- end of HAB API updates ------------*/

int authenticate_image(uint32_t ddr_start, uint32_t image_size); diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index f878b7b..6367562 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -70,7 +70,6 @@ ((hab_rvt_exit_t *)HAB_RVT_EXIT) \ )

-#define IVT_SIZE 0x20 #define ALIGN_SIZE 0x1000 #define CSF_PAD_SIZE 0x2000 #define MX6DQ_PU_IROM_MMU_EN_VAR 0x009024a8

u-boot command "hab_auth_img" tells a user that it takes

- addr - image hex length - offset - hex offset of IVT in the image

but in fact the callback hab_auth_img makes to authenticate_image treats the second 'offset' parameter as an image length.

Furthermore existing code requires the IVT header to be appended to the end of the image which is not actually a requirement of HABv4.

This patch fixes this situation by

1: Adding a new parameter to hab_auth_img - addr : image hex address - length : total length of the image - offset : offset of IVT from addr

2: Updates the existing call into authenticate_image() in arch/arm/mach-imx/spl.c:jump_to_image_no_args() to pass addr, length and IVT offset respectively.

This allows then hab_auth_img to actually operate the way it was specified in the help text and should still allow existing code to work.

It has the added advantage that the IVT header doesn't have to be appended to an image given to HAB - it can be prepended for example.

Note prepending the IVT is what u-boot will do when making an IVT for the BootROM. It should be possible for u-boot properly authenticate images made by mkimage via HAB.

This patch is the first step in making that happen subsequent patches will focus on removing hard-coded offsets to the IVT, which again is not mandated to live at the end of a .imx image.

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/include/asm/mach-imx/hab.h | 3 +- arch/arm/mach-imx/hab.c | 73 +++++++++++-------------------------- arch/arm/mach-imx/spl.c | 35 +++++++++++++++++- 3 files changed, 57 insertions(+), 54 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h index 91dda42..b2a8031 100644 --- a/arch/arm/include/asm/mach-imx/hab.h +++ b/arch/arm/include/asm/mach-imx/hab.h @@ -148,6 +148,7 @@ typedef void hapi_clock_init_t(void);

/* ----------- end of HAB API updates ------------*/

-int authenticate_image(uint32_t ddr_start, uint32_t image_size); +int authenticate_image(uint32_t ddr_start, uint32_t image_size, + uint32_t ivt_offset);

#endif diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 039a017..2a40d06 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -78,37 +78,6 @@ (is_soc_type(MXC_SOC_MX7ULP) ? 0x80000000 : \ (is_soc_type(MXC_SOC_MX7) ? 0x2000000 : 0x2))

-/* - * +------------+ 0x0 (DDR_UIMAGE_START) - - * | Header | | - * +------------+ 0x40 | - * | | | - * | | | - * | | | - * | | | - * | Image Data | | - * . | | - * . | > Stuff to be authenticated ----+ - * . | | | - * | | | | - * | | | | - * +------------+ | | - * | | | | - * | Fill Data | | | - * | | | | - * +------------+ Align to ALIGN_SIZE | | - * | IVT | | | - * +------------+ + IVT_SIZE - | - * | | | - * | CSF DATA | <---------------------------------------------------------+ - * | | - * +------------+ - * | | - * | Fill Data | - * | | - * +------------+ + CSF_PAD_SIZE - */ - static bool is_hab_enabled(void);

#if !defined(CONFIG_SPL_BUILD) @@ -361,20 +330,22 @@ int do_hab_status(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) static int do_authenticate_image(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { - ulong addr, ivt_offset; + ulong addr, length, ivt_offset; int rcode = 0;

- if (argc < 3) + if (argc < 4) return CMD_RET_USAGE;

addr = simple_strtoul(argv[1], NULL, 16); - ivt_offset = simple_strtoul(argv[2], NULL, 16); + length = simple_strtoul(argv[2], NULL, 16); + ivt_offset = simple_strtoul(argv[3], NULL, 16);

- rcode = authenticate_image(addr, ivt_offset); + rcode = authenticate_image(addr, length, ivt_offset); if (rcode == 0) rcode = CMD_RET_SUCCESS; else rcode = CMD_RET_FAILURE; + return rcode; }

@@ -385,10 +356,11 @@ U_BOOT_CMD( );

U_BOOT_CMD( - hab_auth_img, 3, 0, do_authenticate_image, + hab_auth_img, 4, 0, do_authenticate_image, "authenticate image via HAB", - "addr ivt_offset\n" + "addr length ivt_offset\n" "addr - image hex address\n" + "length - image hex length\n" "ivt_offset - hex offset of IVT in the image" );

@@ -411,11 +383,12 @@ static bool is_hab_enabled(void) return (reg & IS_HAB_ENABLED_BIT) == IS_HAB_ENABLED_BIT; }

-int authenticate_image(uint32_t ddr_start, uint32_t image_size) +int authenticate_image(uint32_t ddr_start, uint32_t image_size, + uint32_t ivt_offset) { uint32_t load_addr = 0; size_t bytes; - ptrdiff_t ivt_offset = 0; + uint32_t ivt_addr = 0; int result = 1; ulong start; hab_rvt_authenticate_image_t *hab_rvt_authenticate_image; @@ -441,24 +414,18 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size) goto hab_caam_clock_disable; }

- /* If not already aligned, Align to ALIGN_SIZE */ - ivt_offset = (image_size + ALIGN_SIZE - 1) & - ~(ALIGN_SIZE - 1); - + /* Calculate IVT address header */ + ivt_addr = ddr_start + ivt_offset; start = ddr_start; - bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE; + bytes = image_size; #ifdef DEBUG - printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", - ivt_offset, ddr_start + ivt_offset); + printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", ivt_offset, ivt_addr); puts("Dumping IVT\n"); - print_buffer(ddr_start + ivt_offset, - (void *)(ddr_start + ivt_offset), - 4, 0x8, 0); + print_buffer(ivt_addr, (void *)(ivt_addr), 4, 0x8, 0);

puts("Dumping CSF Header\n"); - print_buffer(ddr_start + ivt_offset + IVT_SIZE, - (void *)(ddr_start + ivt_offset + IVT_SIZE), - 4, 0x10, 0); + print_buffer(ivt_addr + IVT_SIZE, (void *)(ivt_addr + IVT_SIZE), 4, + 0x10, 0);

#if !defined(CONFIG_SPL_BUILD) get_hab_status(); @@ -468,6 +435,8 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size) printf("\tivt_offset = 0x%x\n", ivt_offset); printf("\tstart = 0x%08lx\n", start); printf("\tbytes = 0x%x\n", bytes); +#else + (void)ivt_addr; #endif /* * If the MMU is enabled, we have to notify the ROM diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index 6e930b3..e5d0c35 100644 --- a/arch/arm/mach-imx/spl.c +++ b/arch/arm/mach-imx/spl.c @@ -152,9 +152,41 @@ u32 spl_boot_mode(const u32 boot_device)

#if defined(CONFIG_SECURE_BOOT)

+/* + * +------------+ 0x0 (DDR_UIMAGE_START) - + * | Header | | + * +------------+ 0x40 | + * | | | + * | | | + * | | | + * | | | + * | Image Data | | + * . | | + * . | > Stuff to be authenticated ----+ + * . | | | + * | | | | + * | | | | + * +------------+ | | + * | | | | + * | Fill Data | | | + * | | | | + * +------------+ Align to ALIGN_SIZE | | + * | IVT | | | + * +------------+ + IVT_SIZE - | + * | | | + * | CSF DATA | <---------------------------------------------------------+ + * | | + * +------------+ + * | | + * | Fill Data | + * | | + * +------------+ + CSF_PAD_SIZE + */ + __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) { typedef void __noreturn (*image_entry_noargs_t)(void); + uint32_t offset;

image_entry_noargs_t image_entry = (image_entry_noargs_t)(unsigned long)spl_image->entry_point; @@ -163,8 +195,9 @@ __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)

/* HAB looks for the CSF at the end of the authenticated data therefore, * we need to subtract the size of the CSF from the actual filesize */ + offset = spl_image->size - CONFIG_CSF_SIZE; if (!authenticate_image(spl_image->load_addr, - spl_image->size - CONFIG_CSF_SIZE)) { + offset + IVT_SIZE + CSF_PAD_SIZE, offset)) { image_entry(); } else { puts("spl: ERROR: image authentication unsuccessful\n");

The i.MX6 has some pretty explicit code associated with informing the IROM about flushing caches during authenticate_image().

Looking at various pieces of documentation its pretty clear the i.MX6 IROM registers are not documented and absent similar documentation on the i.MX7 the next-best fix is to disabled the dcache while making an authenticate_image() callback.

This patch therefore disables dcache temporarily while doing an IROM authenticate_image() callback, thus resolving a lockup encountered in a complex set of authenticate-image calls observed.

Note there is no appreciable performance impact with dcache switched off so this fix is relatively pain-free.

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/mach-imx/hab.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 2a40d06..1d7b069 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -466,10 +466,25 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size, } }

+ /* + * FIXME: Need to disable dcache on MX7 is there an IROM + * register like on MX6 above ? Certain images called in certain + * orders with the dcache switched on will cause + * authenticate_image() to lockup. Switching off the dcache + * resolves the issue. + * https://community.nxp.com/message/953261 + */ + if (is_soc_type(MXC_SOC_MX7)) + dcache_disable(); + load_addr = (uint32_t)hab_rvt_authenticate_image( HAB_CID_UBOOT, ivt_offset, (void **)&start, (size_t *)&bytes, NULL); + + if (is_soc_type(MXC_SOC_MX7)) + dcache_enable(); + if (hab_rvt_exit() != HAB_SUCCESS) { puts("hab exit function fail\n"); load_addr = 0;

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/mach-imx/hab.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 1d7b069..cb6214d 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -80,6 +80,31 @@

static bool is_hab_enabled(void);

+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; +} + #if !defined(CONFIG_SPL_BUILD)

#define MAX_RECORD_BYTES (8*1024) /* 4 kbytes */ @@ -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 @@ -435,8 +469,6 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size, printf("\tivt_offset = 0x%x\n", ivt_offset); printf("\tstart = 0x%08lx\n", start); printf("\tbytes = 0x%x\n", bytes); -#else - (void)ivt_addr; #endif /* * If the MMU is enabled, we have to notify the ROM

The IVT gives the absolute address of the CSF. There is no requirement for the CSF to be located adjacent to the IVT so lets use the address provided in the IVT header instead of the fixed CSF offset currently in place.

Its worth noting if you use u-boot mkimage and the i.MX CST tool as described in the NXP documentation you will get an image like

IVT | BINARY | CSF not IVT | CSF | BINARY as the code currently assumes. The IVT header must correctly describe the location of the CSF or the BootROM will reject it - so the current dependence on a fixed offset is nothing except limiting.

Fix it now.

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/mach-imx/hab.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 479ed96..d620524 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -465,8 +465,7 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size, print_buffer(ivt_addr, (void *)(ivt_addr), 4, 0x8, 0);

puts("Dumping CSF Header\n"); - print_buffer(ivt_addr + IVT_SIZE, (void *)(ivt_addr + IVT_SIZE), 4, - 0x10, 0); + print_buffer(ivt->csf, (void *)(ivt->csf), 4, 0x10, 0);

#if !defined(CONFIG_SPL_BUILD) get_hab_status();

The hab_rvt_check_target() callback according to the HABv4 documentation:

"This function reports whether or not a given target region is allowed for either peripheral configuration or image loading in memory. It is intended for use by post-ROM boot stage components, via the ROM Vector Table, in order to avoid configuring security-sensitive peripherals, or loading images over sensitive memory regions or outside recognized memory devices in the address map."

It is a useful function to support as a precursor to calling into authenticate_image() to validate the target memory region is good.

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/include/asm/mach-imx/hab.h | 9 +++++++++ 1 file changed, 9 insertions(+)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h index 28cde38..14e1220 100644 --- a/arch/arm/include/asm/mach-imx/hab.h +++ b/arch/arm/include/asm/mach-imx/hab.h @@ -113,6 +113,12 @@ enum hab_context { HAB_CTX_MAX };

+enum hab_target { + HAB_TGT_MEMORY = 0x0f, + HAB_TGT_PERIPHERAL = 0xf0, + HAB_TGT_ANY = 0x55, +}; + struct imx_sec_config_fuse_t { int bank; int word; @@ -132,6 +138,8 @@ typedef enum hab_status hab_rvt_entry_t(void); typedef enum hab_status hab_rvt_exit_t(void); typedef void *hab_rvt_authenticate_image_t(uint8_t, ptrdiff_t, void **, size_t *, hab_loader_callback_f_t); +typedef enum hab_status hab_rvt_check_target_t(enum hab_target, const void *, + size_t); typedef void hapi_clock_init_t(void);

#define HAB_ENG_ANY 0x00 /* Select first compatible engine */ @@ -158,6 +166,7 @@ typedef void hapi_clock_init_t(void);

#define HAB_RVT_ENTRY (*(uint32_t *)(HAB_RVT_BASE + 0x04)) #define HAB_RVT_EXIT (*(uint32_t *)(HAB_RVT_BASE + 0x08)) +#define HAB_RVT_CHECK_TARGET (*(uint32_t *)(HAB_RVT_BASE + 0x0C)) #define HAB_RVT_AUTHENTICATE_IMAGE (*(uint32_t *)(HAB_RVT_BASE + 0x10)) #define HAB_RVT_REPORT_EVENT (*(uint32_t *)(HAB_RVT_BASE + 0x20)) #define HAB_RVT_REPORT_STATUS (*(uint32_t *)(HAB_RVT_BASE + 0x24))

Add a hab_rvt_check_target() step to authenticate_image() as a sanity check for the target memory region authenticate_image() will run over, prior to making the BootROM authentication callback itself.

This check is recommended by the HAB documentation so it makes sense to adhere to the guidance and perform that check as directed.

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/mach-imx/hab.c | 10 ++++++++++ 1 file changed, 10 insertions(+)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index eb18f76..864b1e2 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -437,12 +437,15 @@ 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; + hab_rvt_check_target_t *hab_rvt_check_target; struct ivt *ivt; struct ivt_header *ivt_hdr; + enum hab_status status;

hab_rvt_authenticate_image = hab_rvt_authenticate_image_p; hab_rvt_entry = hab_rvt_entry_p; hab_rvt_exit = hab_rvt_exit_p; + hab_rvt_check_target = hab_rvt_check_target_p;

if (!is_hab_enabled()) { puts("hab fuse not enabled\n"); @@ -477,6 +480,13 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,

start = ddr_start; bytes = image_size; + + status = hab_rvt_check_target(HAB_TGT_MEMORY, (void *)ddr_start, bytes); + if (status != HAB_SUCCESS) { + printf("HAB check target 0x%08x-0x%08x fail\n", + ddr_start, ddr_start + bytes); + goto hab_caam_clock_disable; + } #ifdef DEBUG printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", ivt_offset, ivt_addr); printf("ivt entry = 0x%08x, dcd = 0x%08x, csf = 0x%08x\n", ivt->entry,

Tidy up the HAB namespace a bit by prefixing external functions with imx_hab. All external facing functions past this point will be prefixed in the same way to make the fact we are doing IMX HAB activities clear from reading the code. authenticate_image() could mean anything imx_hab_authenticate_image() is on the other hand very explicit.

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/include/asm/mach-imx/hab.h | 4 ++-- arch/arm/mach-imx/hab.c | 6 +++--- arch/arm/mach-imx/spl.c | 5 +++-- 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h index 14e1220..98bc1bd 100644 --- a/arch/arm/include/asm/mach-imx/hab.h +++ b/arch/arm/include/asm/mach-imx/hab.h @@ -185,7 +185,7 @@ typedef void hapi_clock_init_t(void);

/* ----------- end of HAB API updates ------------*/

-int authenticate_image(uint32_t ddr_start, uint32_t image_size, - uint32_t ivt_offset); +int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size, + uint32_t ivt_offset);

#endif diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 098cd3c..5c2d5fd 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -392,7 +392,7 @@ static int do_authenticate_image(cmd_tbl_t *cmdtp, int flag, int argc, length = simple_strtoul(argv[2], NULL, 16); ivt_offset = simple_strtoul(argv[3], NULL, 16);

- rcode = authenticate_image(addr, length, ivt_offset); + rcode = imx_hab_authenticate_image(addr, length, ivt_offset); if (rcode == 0) rcode = CMD_RET_SUCCESS; else @@ -435,8 +435,8 @@ static bool is_hab_enabled(void) return (reg & IS_HAB_ENABLED_BIT) == IS_HAB_ENABLED_BIT; }

-int authenticate_image(uint32_t ddr_start, uint32_t image_size, - uint32_t ivt_offset) +int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size, + uint32_t ivt_offset) { uint32_t load_addr = 0; size_t bytes; diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index e5d0c35..a5478ce 100644 --- a/arch/arm/mach-imx/spl.c +++ b/arch/arm/mach-imx/spl.c @@ -196,8 +196,9 @@ __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) /* HAB looks for the CSF at the end of the authenticated data therefore, * we need to subtract the size of the CSF from the actual filesize */ offset = spl_image->size - CONFIG_CSF_SIZE; - if (!authenticate_image(spl_image->load_addr, - offset + IVT_SIZE + CSF_PAD_SIZE, offset)) { + if (!imx_hab_authenticate_image(spl_image->load_addr, + offset + IVT_SIZE + CSF_PAD_SIZE, + offset)) { image_entry(); } else { puts("spl: ERROR: image authentication unsuccessful\n");

It will be helpful to boot commands to know if the HAB is enabled. Export imx_hab_is_enabled() now to facilitate further work with this data-point in a secure-boot context.

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/include/asm/mach-imx/hab.h | 1 + arch/arm/mach-imx/hab.c | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h index 98bc1bd..5c13aff 100644 --- a/arch/arm/include/asm/mach-imx/hab.h +++ b/arch/arm/include/asm/mach-imx/hab.h @@ -187,5 +187,6 @@ typedef void hapi_clock_init_t(void);

int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size, uint32_t ivt_offset); +bool imx_hab_is_enabled(void);

#endif diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index a1a13d6..7b50ef5 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -96,8 +96,6 @@ static inline enum hab_status hab_rvt_check_target_new(enum hab_target target, (is_soc_type(MXC_SOC_MX7ULP) ? 0x80000000 : \ (is_soc_type(MXC_SOC_MX7) ? 0x2000000 : 0x2))

-static bool imx_hab_is_enabled(void); - 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, @@ -419,7 +417,7 @@ U_BOOT_CMD(

#endif /* !defined(CONFIG_SPL_BUILD) */

-static bool imx_hab_is_enabled(void) +bool imx_hab_is_enabled(void) { struct imx_sec_config_fuse_t *fuse = (struct imx_sec_config_fuse_t *)&imx_sec_config_fuse;

This patch implements the basic callback hooks for hab_rvt_check_failsafe for BootROM code using the older BootROM address layout - in my test case the i.MX7. Code based on new BootROM callbacks will just do nothing and there's definitely a TODO to implement that extra functionality on the alternative BootROM API.

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 Cc: Breno Matheus Lima brenomatheus@gmail.com --- arch/arm/mach-imx/hab.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 7b50ef5..01c0853 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -70,6 +70,21 @@ ((hab_rvt_exit_t *)HAB_RVT_EXIT) \ )

+static inline void hab_rvt_failsafe_new(void) +{ +} + +#define hab_rvt_failsafe_p \ +( \ + (is_mx6dqp()) ? \ + ((hab_rvt_failsafe_t *)hab_rvt_failsafe_new) : \ + (is_mx6dq() && (soc_rev() >= CHIP_REV_1_5)) ? \ + ((hab_rvt_failsafe_t *)hab_rvt_failsafe_new) : \ + (is_mx6sdl() && (soc_rev() >= CHIP_REV_1_2)) ? \ + ((hab_rvt_failsafe_t *)hab_rvt_failsafe_new) : \ + ((hab_rvt_failsafe_t *)HAB_RVT_FAILSAFE) \ +) + static inline enum hab_status hab_rvt_check_target_new(enum hab_target target, const void *start, size_t bytes)