Commit 1fb1eb7beac84169c2ce6a70d5815cdac07385cb

Authored by Utkarsh Gupta
Committed by Ye Li
1 parent 5c156812fb

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 : [<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>
(cherry picked from commit dabffd1b04df3b0393ef6a9a35b5fd816edd8c63)

Showing 2 changed files with 56 additions and 12 deletions Side-by-side Diff

arch/arm/imx-common/hab.c
... ... @@ -643,6 +643,54 @@
643 643 return 1;
644 644 }
645 645  
  646 +/*
  647 + * Validate IVT structure of the image being authenticated
  648 + */
  649 +static int validate_ivt(int ivt_offset, ulong start_addr)
  650 +{
  651 + const struct hab_ivt *ivt_initial = NULL;
  652 + const uint8_t *start = (const uint8_t *)start_addr;
  653 +
  654 + if (start_addr & 0x3) {
  655 + puts("Error: Image's start address is not 4 byte aligned\n");
  656 + return 0;
  657 + }
  658 +
  659 + ivt_initial = (const struct hab_ivt *)(start + ivt_offset);
  660 +
  661 + const struct hab_hdr *ivt_hdr = &ivt_initial->hdr;
  662 +
  663 + /* Check IVT fields before allowing authentication */
  664 + if ((ivt_hdr->tag == HAB_TAG_IVT && \
  665 + ((ivt_hdr->len[0] << 8) + ivt_hdr->len[1]) == IVT_HDR_LEN && \
  666 + (ivt_hdr->par & HAB_MAJ_MASK) == HAB_MAJ_VER) && \
  667 + (ivt_initial->entry != 0x0) && \
  668 + (ivt_initial->reserved1 == 0x0) && \
  669 + (ivt_initial->self == (uint32_t)ivt_initial) && \
  670 + (ivt_initial->csf != 0x0) && \
  671 + (ivt_initial->reserved2 == 0x0)) {
  672 + /* Report boot failure if DCD pointer is found in IVT */
  673 + if (ivt_initial->dcd != 0x0)
  674 + puts("Error: DCD pointer must be 0\n");
  675 + else
  676 + return 1;
  677 + }
  678 +
  679 + puts("Error: Invalid IVT structure\n");
  680 + puts("\nAllowed IVT structure:\n");
  681 + puts("IVT HDR = 0x4X2000D1\n");
  682 + puts("IVT ENTRY = 0xXXXXXXXX\n");
  683 + puts("IVT RSV1 = 0x0\n");
  684 + puts("IVT DCD = 0x0\n"); /* Recommended */
  685 + puts("IVT BOOT_DATA = 0xXXXXXXXX\n"); /* Commonly 0x0 */
  686 + puts("IVT SELF = 0xXXXXXXXX\n"); /* = ddr_start + ivt_offset */
  687 + puts("IVT CSF = 0xXXXXXXXX\n");
  688 + puts("IVT RSV2 = 0x0\n");
  689 +
  690 + /* Invalid IVT structure */
  691 + return 0;
  692 +}
  693 +
646 694 uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size)
647 695 {
648 696 ulong load_addr = 0;
... ... @@ -665,6 +713,10 @@
665 713 start = ddr_start;
666 714 bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE;
667 715  
  716 + /* Validate IVT structure */
  717 + if (!validate_ivt(ivt_offset, start))
  718 + return result;
  719 +
668 720 puts("Check CSF for Write Data command before ");
669 721 puts("authenticating image\n");
670 722 if (!csf_is_valid(ivt_offset, start, bytes))
... ... @@ -722,18 +774,6 @@
722 774 }
723 775 }
724 776 #endif
725   -
726   - /* Report boot failure if DCD pointer is found in IVT */
727   - unsigned char *dcd_ptr = (unsigned char *)(ddr_start + ivt_offset + 0xC);
728   -
729   - do {
730   - if (*dcd_ptr) {
731   - puts("Error: DCD pointer must be 0\n");
732   - return result;
733   - }
734   - dcd_ptr++;
735   - } while (dcd_ptr < (unsigned char *)(ddr_start + ivt_offset + 0x10));
736   -
737 777 load_addr = (ulong)hab_rvt_authenticate_image(
738 778 HAB_CID_UBOOT,
739 779 ivt_offset, (void **)&start,
arch/arm/include/asm/imx-common/hab.h
... ... @@ -177,6 +177,10 @@
177 177 ((size_t)(((const struct hab_hdr *)&(hdr))->len[0] << 8) \
178 178 + (size_t)((const struct hab_hdr *)&(hdr))->len[1])
179 179  
  180 +#define HAB_TAG_IVT 0xD1
  181 +#define IVT_HDR_LEN 0x20
  182 +#define HAB_MAJ_VER 0x40
  183 +#define HAB_MAJ_MASK 0xF0
180 184 /* ----------- end of HAB API updates ------------*/
181 185  
182 186 uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size);