Hi Bryan,
2017-12-28 16:49 GMT-02:00 Bryan O'Donoghue bryan.odonoghue@linaro.org:
The current code disjoins an entire block of code on hab_entry pass/fail resulting in a large chunk of authenticate_image being offset to the right.
Fix this by checking hab_entry() pass/failure and exiting the function directly if in an error state.
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 | 118 ++++++++++++++++++++++++------------------------ 1 file changed, 60 insertions(+), 58 deletions(-)
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 6f86c02..f878b7b 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -438,75 +438,77 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size)
hab_caam_clock_enable(1);
if (hab_rvt_entry() == HAB_SUCCESS) {/* If not already aligned, Align to ALIGN_SIZE */ivt_offset = (image_size + ALIGN_SIZE - 1) &~(ALIGN_SIZE - 1);
if (hab_rvt_entry() != HAB_SUCCESS) {puts("hab entry function fail\n");goto hab_caam_clock_disable;}
start = ddr_start;bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE;
/* 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; }+hab_caam_clock_disable: hab_caam_clock_enable(0);
#if !defined(CONFIG_SPL_BUILD)
Just a suggestion here, can you please check if it's possible to move "hab_caam_clock_disable:" after the "#if !defined(CONFIG_SPL_BUILD)" branch?
I'm authenticating a Kernel image with your patch set applied:
=> fatload mmc 0:1 0x12000000 zImage reading zImage 7057248 bytes read in 355 ms (19 MiB/s) => hab_auth_img 0x12000000 0x6baf60 0x6ba000
Authenticate image from DDR location 0x12000000...
ivt_offset = 0x6ba000, ivt addr = 0x126ba000 ivt entry = 0x12000000, dcd = 0x00000000, csf = 0x126ba020 Dumping IVT 126ba000: 412000d1 12000000 00000000 00000000 .. A............ 126ba010: 00000000 126ba000 126ba020 00000000 ......k. .k..... Dumping CSF Header 126ba020: 425000d4 000c00be 00001703 50000000 ..PB...........P 126ba030: 020c00be 01000009 90040000 000c00ca ................ 126ba040: 001dc501 e4070000 000c00be 02000009 ................ 126ba050: e8090000 001400ca 001dc502 3c0d0000 ...............<
Secure boot enabled
HAB Configuration: 0xcc, HAB State: 0x99 No HAB Events Found!
Calling authenticate_image in ROM ivt_offset = 0x6ba000 start = 0x12000000 bytes = 0x6baf60
Secure boot enabled
HAB Configuration: 0xcc, HAB State: 0x99 No HAB Events Found!
Then I'm modifying the content in ivt->self for generating an error:
=> mw 0x126ba014 0x126aa030 => hab_auth_img 0x12000000 0x6baf60 0x6ba000
Authenticate image from DDR location 0x12000000... ivt->self 0x126aa030 pointer is 0x126ba000
Secure boot enabled
HAB Configuration: 0xcc, HAB State: 0x99 No HAB Events Found!
=>
In this situation the "hab_rvt_authenticate_image()" is not executed, It's a bit confusing to receive a "No HAB Events Found!" message after running hab_auth_img on this case.
Thanks, Breno Lima