From: Utkarsh Gupta utkarsh.gupta@nxp.com
Calling csf_is_valid() with an un-signed image may lead to data abort as the CSF pointer could be pointing to a garbage address when accessed in HAB_HDR_LEN(*(const struct hab_hdr *)(ulong)ivt_initial->csf).
Authenticate image from DDR location 0x80800000... Check CSF for Write Data command before authenticating image data abort pc : [<fff5494c>] lr : [<fff54910>] reloc pc : [<8780294c>] lr : [<87802910>] sp : fdf45dc8 ip : 00000214 fp : 00000000 r10: fffb6170 r9 : fdf4fec0 r8 : 00722020 r7 : 80f20000 r6 : 80800000 r5 : 80800000 r4 : 00720000 r3 : 17a5aca3 r2 : 00000000 r1 : 80f2201f r0 : 00000019 Flags: NzcV IRQs off FIQs off Mode SVC_32 Resetting CPU ...
resetting ...
To avoid such errors during authentication process, validate IVT structure by calling validate_ivt function which checks the following values in an IVT:
IVT_HEADER = 0x4X2000D1 ENTRY != 0x0 RES1 = 0x0 DCD = 0x0 /* Recommended */ SELF != 0x0 /* Absoulute address of IVT */ CSF != 0x0 RES2 = 0x0
This commit also checks if Image's start address is 4 byte aligned.
commit "0088d127 MLK-14945 HAB: Check if IVT valid before authenticating image" removed as this patch addresses the issue.
Signed-off-by: Utkarsh Gupta utkarsh.gupta@nxp.com Signed-off-by: Ye Li ye.li@nxp.com Signed-off-by: Peng Fan peng.fan@nxp.com --- arch/arm/mach-imx/hab.c | 59 ++++++++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 16 deletions(-)
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 9ebdbe6ac3..f2d5f744e8 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -728,6 +728,48 @@ static bool csf_is_valid(struct ivt *ivt, ulong start_addr, size_t bytes) return true; }
+/* + * Validate IVT structure of the image being authenticated + */ +static int validate_ivt(struct ivt *ivt_initial) +{ + struct ivt_header *ivt_hdr = &ivt_initial->hdr; + + if ((ulong)ivt_initial & 0x3) { + puts("Error: Image's start address is not 4 byte aligned\n"); + return 0; + } + + /* Check IVT fields before allowing authentication */ + if ((!verify_ivt_header(ivt_hdr)) && \ + (ivt_initial->entry != 0x0) && \ + (ivt_initial->reserved1 == 0x0) && \ + (ivt_initial->self == \ + (uint32_t)((ulong)ivt_initial & 0xffffffff)) && \ + (ivt_initial->csf != 0x0) && \ + (ivt_initial->reserved2 == 0x0)) { + /* Report boot failure if DCD pointer is found in IVT */ + if (ivt_initial->dcd != 0x0) + puts("Error: DCD pointer must be 0\n"); + else + return 1; + } + + puts("Error: Invalid IVT structure\n"); + debug("\nAllowed IVT structure:\n"); + debug("IVT HDR = 0x4X2000D1\n"); + debug("IVT ENTRY = 0xXXXXXXXX\n"); + debug("IVT RSV1 = 0x0\n"); + debug("IVT DCD = 0x0\n"); /* Recommended */ + debug("IVT BOOT_DATA = 0xXXXXXXXX\n"); /* Commonly 0x0 */ + debug("IVT SELF = 0xXXXXXXXX\n"); /* = ddr_start + ivt_offset */ + debug("IVT CSF = 0xXXXXXXXX\n"); + debug("IVT RSV2 = 0x0\n"); + + /* Invalid IVT structure */ + return 0; +} + bool imx_hab_is_enabled(void) { struct imx_sec_config_fuse_t *fuse = @@ -753,7 +795,6 @@ int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size, int result = 1; ulong start; struct ivt *ivt; - struct ivt_header *ivt_hdr; enum hab_status status;
if (!imx_hab_is_enabled()) { @@ -769,24 +810,10 @@ int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size, /* Calculate IVT address header */ ivt_addr = (ulong) (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_authentication_exit; - - /* Verify IVT body */ - if (ivt->self != ivt_addr) { - printf("ivt->self 0x%08x pointer is 0x%08lx\n", - ivt->self, ivt_addr); + if (!validate_ivt(ivt)) goto hab_authentication_exit; - } - - /* Verify if IVT DCD pointer is NULL */ - if (ivt->dcd) { - puts("Error: DCD pointer must be NULL\n"); - goto hab_authentication_exit; - }
start = ddr_start; bytes = image_size;