From 1fb1eb7beac84169c2ce6a70d5815cdac07385cb Mon Sep 17 00:00:00 2001 From: Utkarsh Gupta Date: Mon, 2 Apr 2018 19:41:16 -0500 Subject: [PATCH] MLK-17935: imx: HAB: Validate IVT before authenticating image 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 : [] lr : [] 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 (cherry picked from commit dabffd1b04df3b0393ef6a9a35b5fd816edd8c63) --- arch/arm/imx-common/hab.c | 64 ++++++++++++++++++++++----- arch/arm/include/asm/imx-common/hab.h | 4 ++ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/arch/arm/imx-common/hab.c b/arch/arm/imx-common/hab.c index d646b8ae91..14a5e45e76 100644 --- a/arch/arm/imx-common/hab.c +++ b/arch/arm/imx-common/hab.c @@ -643,6 +643,54 @@ static int csf_is_valid(int ivt_offset, ulong start_addr, size_t bytes) return 1; } +/* + * Validate IVT structure of the image being authenticated + */ +static int validate_ivt(int ivt_offset, ulong start_addr) +{ + const struct hab_ivt *ivt_initial = NULL; + const uint8_t *start = (const uint8_t *)start_addr; + + if (start_addr & 0x3) { + puts("Error: Image's start address is not 4 byte aligned\n"); + return 0; + } + + ivt_initial = (const struct hab_ivt *)(start + ivt_offset); + + const struct hab_hdr *ivt_hdr = &ivt_initial->hdr; + + /* Check IVT fields before allowing authentication */ + if ((ivt_hdr->tag == HAB_TAG_IVT && \ + ((ivt_hdr->len[0] << 8) + ivt_hdr->len[1]) == IVT_HDR_LEN && \ + (ivt_hdr->par & HAB_MAJ_MASK) == HAB_MAJ_VER) && \ + (ivt_initial->entry != 0x0) && \ + (ivt_initial->reserved1 == 0x0) && \ + (ivt_initial->self == (uint32_t)ivt_initial) && \ + (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"); + puts("\nAllowed IVT structure:\n"); + puts("IVT HDR = 0x4X2000D1\n"); + puts("IVT ENTRY = 0xXXXXXXXX\n"); + puts("IVT RSV1 = 0x0\n"); + puts("IVT DCD = 0x0\n"); /* Recommended */ + puts("IVT BOOT_DATA = 0xXXXXXXXX\n"); /* Commonly 0x0 */ + puts("IVT SELF = 0xXXXXXXXX\n"); /* = ddr_start + ivt_offset */ + puts("IVT CSF = 0xXXXXXXXX\n"); + puts("IVT RSV2 = 0x0\n"); + + /* Invalid IVT structure */ + return 0; +} + uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size) { ulong load_addr = 0; @@ -665,6 +713,10 @@ uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size) start = ddr_start; bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE; + /* Validate IVT structure */ + if (!validate_ivt(ivt_offset, start)) + return result; + puts("Check CSF for Write Data command before "); puts("authenticating image\n"); if (!csf_is_valid(ivt_offset, start, bytes)) @@ -722,18 +774,6 @@ uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size) } } #endif - - /* Report boot failure if DCD pointer is found in IVT */ - unsigned char *dcd_ptr = (unsigned char *)(ddr_start + ivt_offset + 0xC); - - do { - if (*dcd_ptr) { - puts("Error: DCD pointer must be 0\n"); - return result; - } - dcd_ptr++; - } while (dcd_ptr < (unsigned char *)(ddr_start + ivt_offset + 0x10)); - load_addr = (ulong)hab_rvt_authenticate_image( HAB_CID_UBOOT, ivt_offset, (void **)&start, diff --git a/arch/arm/include/asm/imx-common/hab.h b/arch/arm/include/asm/imx-common/hab.h index a2476ad287..374af87b2d 100644 --- a/arch/arm/include/asm/imx-common/hab.h +++ b/arch/arm/include/asm/imx-common/hab.h @@ -177,6 +177,10 @@ typedef void hapi_clock_init_t(void); ((size_t)(((const struct hab_hdr *)&(hdr))->len[0] << 8) \ + (size_t)((const struct hab_hdr *)&(hdr))->len[1]) +#define HAB_TAG_IVT 0xD1 +#define IVT_HDR_LEN 0x20 +#define HAB_MAJ_VER 0x40 +#define HAB_MAJ_MASK 0xF0 /* ----------- end of HAB API updates ------------*/ uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size); -- 2.17.1