From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web10.9654.1624878186806875176 for ; Mon, 28 Jun 2021 04:03:07 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@posteo.de header.s=2017 header.b=FJ+Jg+ic; spf=pass (domain: posteo.de, ip: 185.67.36.65, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout01.posteo.de (Postfix) with ESMTPS id 0261424002B for ; Mon, 28 Jun 2021 13:03:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1624878185; bh=IT70ud+x3c3I6yRJEdXXVhvQl5Pz3OguOqUp31MOQvM=; h=Subject:To:Cc:From:Date:From; b=FJ+Jg+ic7yPgfUa75OkUraj48LjYx0yIn4p7zKAcXdSOzHB3pSuu2tHsWR9AHkv8a +sE2kdSyc4Pgn91cVcamQz4jpOo476L6M8mM3/+yomfY7Wmofyg7VLaZJnINW3t5sb IrmsPXCGb1kJTpQdm6C43+FS0wYFtdZEXCTRyHZwhWaaM/rfv3Sr9m9fC7mOspd9kF qEy/U/TemJnfSfy/nxYabDAVLFjRUVHc5H6amIW+5GGu3NEL0cBf0FscPoPT7/FKry X4FnUxbaEcWiY1jfvfL6ljOnBREcY8kmuFECe6iVRqCz/GUp+UCu2HfQM49YkiYRmI WBiNA5szgZG1A== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4GD4TM5BBsz9rxM; Mon, 28 Jun 2021 13:03:01 +0200 (CEST) Subject: Re: [edk2-devel] [PATCH] UefiPayloadPkg/PayloadLoader: Add more checks to verify ELF images To: devel@edk2.groups.io, ray.ni@intel.com Cc: Guo Dong , Benjamin You References: <20210612060304.43-1-ray.ni@intel.com> From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-ID: <3e932f6b-9ba1-edb7-c1b1-7226cb14fa4b@posteo.de> Date: Mon, 28 Jun 2021 11:03:00 +0000 MIME-Version: 1.0 In-Reply-To: <20210612060304.43-1-ray.ni@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: quoted-printable Hey Ray, Sorry for not having properly checked yet, I definitely plan to still.=20 However, I probably won't till a pointer alignment macro lands (I plan=20 to submit a bunch of things, including this, within the next two weeks).=20 Once it has been merged, I think this patch can be improved with=20 alignment checks. Thanks for your time! Best regards, Marvin On 12.06.21 08:03, Ni, Ray wrote: > More checks are added to verify ELF image. > ParseElfImage() is changed to InitializeElfContext() > > Signed-off-by: Ray Ni > Cc: Marvin H=C3=A4user > Cc: Guo Dong > Cc: Benjamin You > --- > UefiPayloadPkg/PayloadLoaderPeim/ElfLib.h | 11 +- > .../PayloadLoaderPeim/ElfLib/Elf32Lib.c | 38 ++-- > .../PayloadLoaderPeim/ElfLib/Elf64Lib.c | 39 ++-- > .../PayloadLoaderPeim/ElfLib/ElfLib.c | 210 +++++++++++++----= - > .../PayloadLoaderPeim/PayloadLoaderPeim.c | 6 +- > 5 files changed, 188 insertions(+), 116 deletions(-) > > diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib.h b/UefiPayloadPkg= /PayloadLoaderPeim/ElfLib.h > index 9cfc2912cf..0ed93140a9 100644 > --- a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib.h > +++ b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib.h > @@ -17,7 +17,6 @@ > #define ELF_PT_LOAD 1 > > =20 > > typedef struct { > > - RETURN_STATUS ParseStatus; ///< Return the status after = ParseElfImage(). > > UINT8 *FileBase; ///< The source location in = memory. > > UINTN FileSize; ///< The size including sect= ions that don't require loading. > > UINT8 *PreferredImageAddress; ///< The preferred image to = be loaded. No relocation is needed if loaded to this address. > > @@ -45,7 +44,10 @@ typedef struct { > /** > > Parse the ELF image info. > > =20 > > - @param[in] ImageBase Memory address of an image. > > + On return, all fields in ElfCt are updated except ImageAddress. > > + > > + @param[in] FileBase Memory address of an image. > > + @param[in] MaxFileSize The maximum file size. > > @param[out] ElfCt The EFL image context pointer. > > =20 > > @retval EFI_INVALID_PARAMETER Input parameters are not valid. > > @@ -55,8 +57,9 @@ typedef struct { > **/ > > EFI_STATUS > > EFIAPI > > -ParseElfImage ( > > - IN VOID *ImageBase, > > +InitializeElfContext ( > > + IN VOID *FileBase, > > + IN UINTN MaxFileSize, > > OUT ELF_IMAGE_CONTEXT *ElfCt > > ); > > =20 > > diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c b/UefiP= ayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c > index 3fa100ce4a..79f4ce623b 100644 > --- a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c > +++ b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c > @@ -115,7 +115,7 @@ ProcessRelocation32 ( > UINT32 Type; > > =20 > > for ( Index =3D 0 > > - ; RelaEntrySize * Index < RelaSize > > + ; Index < RelaSize / RelaEntrySize > > ; Index++, Rela =3D ELF_NEXT_ENTRY (Elf32_Rela, Rela, RelaEntry= Size) > > ) { > > // > > @@ -137,7 +137,6 @@ ProcessRelocation32 ( > // Dynamic section doesn't contain entries of this type. > > // > > DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", T= ype)); > > - ASSERT (FALSE); > > } else { > > *Ptr +=3D (UINT32) Delta; > > } > > @@ -164,7 +163,6 @@ ProcessRelocation32 ( > // Calculation: B + A > > // > > if (RelaType =3D=3D SHT_RELA) { > > - ASSERT (*Ptr =3D=3D 0); > > *Ptr =3D (UINT32) Delta + Rela->r_addend; > > } else { > > // > > @@ -177,7 +175,6 @@ ProcessRelocation32 ( > // non-Dynamic section doesn't contain entries of this type= . > > // > > DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", T= ype)); > > - ASSERT (FALSE); > > } > > break; > > =20 > > @@ -236,12 +233,12 @@ RelocateElf32Dynamic ( > // > > // It's abnormal a DYN ELF doesn't contain a dynamic section. > > // > > - ASSERT (DynShdr !=3D NULL); > > if (DynShdr =3D=3D NULL) { > > return EFI_UNSUPPORTED; > > } > > - ASSERT (DynShdr->sh_type =3D=3D SHT_DYNAMIC); > > - ASSERT (DynShdr->sh_entsize >=3D sizeof (*Dyn)); > > + if ((DynShdr->sh_type !=3D SHT_DYNAMIC) || DynShdr->sh_entsize < siz= eof (*Dyn)) { > > + return EFI_UNSUPPORTED; > > + } > > =20 > > // > > // 2. Locate the relocation section from the dynamic section. > > @@ -286,9 +283,6 @@ RelocateElf32Dynamic ( > } > > =20 > > if (RelaOffset =3D=3D MAX_UINT64) { > > - ASSERT (RelaCount =3D=3D 0); > > - ASSERT (RelaEntrySize =3D=3D 0); > > - ASSERT (RelaSize =3D=3D 0); > > // > > // It's fine that a DYN ELF doesn't contain relocation section. > > // > > @@ -299,23 +293,22 @@ RelocateElf32Dynamic ( > // Verify the existence of the relocation section. > > // > > RelShdr =3D GetElf32SectionByRange (ElfCt->FileBase, RelaOffset, Re= laSize); > > - ASSERT (RelShdr !=3D NULL); > > if (RelShdr =3D=3D NULL) { > > return EFI_UNSUPPORTED; > > } > > - ASSERT (RelShdr->sh_type =3D=3D RelaType); > > - ASSERT (RelShdr->sh_entsize =3D=3D RelaEntrySize); > > + if ((RelShdr->sh_type !=3D RelaType) || (RelShdr->sh_entsize !=3D Re= laEntrySize)) { > > + return EFI_UNSUPPORTED; > > + } > > =20 > > // > > // 3. Process the relocation section. > > // > > - ProcessRelocation32 ( > > - (Elf32_Rela *) (ElfCt->FileBase + RelShdr->sh_offset), > > - RelShdr->sh_size, RelShdr->sh_entsize, RelShdr->sh_type, > > - (UINTN) ElfCt->ImageAddress - (UINTN) ElfCt->PreferredImageAddress= , > > - TRUE > > - ); > > - return EFI_SUCCESS; > > + return ProcessRelocation32 ( > > + (Elf32_Rela *) (ElfCt->FileBase + RelShdr->sh_offset), > > + RelShdr->sh_size, RelShdr->sh_entsize, RelShdr->sh_type, > > + (UINTN) ElfCt->ImageAddress - (UINTN) ElfCt->PreferredImage= Address, > > + TRUE > > + ); > > } > > =20 > > /** > > @@ -331,7 +324,6 @@ RelocateElf32Sections ( > IN ELF_IMAGE_CONTEXT *ElfCt > > ) > > { > > - EFI_STATUS Status; > > Elf32_Ehdr *Ehdr; > > Elf32_Shdr *RelShdr; > > Elf32_Shdr *Shdr; > > @@ -351,9 +343,7 @@ RelocateElf32Sections ( > // > > if (Ehdr->e_type =3D=3D ET_DYN) { > > DEBUG ((DEBUG_INFO, "DYN ELF: Relocate using dynamic sections...\= n")); > > - Status =3D RelocateElf32Dynamic (ElfCt); > > - ASSERT_EFI_ERROR (Status); > > - return Status; > > + return RelocateElf32Dynamic (ElfCt); > > } > > =20 > > // > > diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c b/UefiP= ayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c > index e364807007..cfe70639ca 100644 > --- a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c > +++ b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c > @@ -115,7 +115,7 @@ ProcessRelocation64 ( > UINT32 Type; > > =20 > > for ( Index =3D 0 > > - ; MultU64x64 (RelaEntrySize, Index) < RelaSize > > + ; Index < DivU64x64Remainder (RelaSize, RelaEntrySize, NULL) > > ; Index++, Rela =3D ELF_NEXT_ENTRY (Elf64_Rela, Rela, RelaEntry= Size) > > ) { > > // > > @@ -138,7 +138,6 @@ ProcessRelocation64 ( > // Dynamic section doesn't contain entries of this type. > > // > > DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", T= ype)); > > - ASSERT (FALSE); > > } else { > > *Ptr +=3D Delta; > > } > > @@ -149,7 +148,6 @@ ProcessRelocation64 ( > // Dynamic section doesn't contain entries of this type. > > // > > DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", Typ= e)); > > - ASSERT (FALSE); > > break; > > =20 > > case R_X86_64_RELATIVE: > > @@ -173,7 +171,6 @@ ProcessRelocation64 ( > // Calculation: B + A > > // > > if (RelaType =3D=3D SHT_RELA) { > > - ASSERT (*Ptr =3D=3D 0); > > *Ptr =3D Delta + Rela->r_addend; > > } else { > > // > > @@ -186,7 +183,6 @@ ProcessRelocation64 ( > // non-Dynamic section doesn't contain entries of this type= . > > // > > DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", T= ype)); > > - ASSERT (FALSE); > > } > > break; > > =20 > > @@ -245,12 +241,12 @@ RelocateElf64Dynamic ( > // > > // It's abnormal a DYN ELF doesn't contain a dynamic section. > > // > > - ASSERT (DynShdr !=3D NULL); > > if (DynShdr =3D=3D NULL) { > > return EFI_UNSUPPORTED; > > } > > - ASSERT (DynShdr->sh_type =3D=3D SHT_DYNAMIC); > > - ASSERT (DynShdr->sh_entsize >=3D sizeof (*Dyn)); > > + if ((DynShdr->sh_type !=3D SHT_DYNAMIC) || DynShdr->sh_entsize < siz= eof (*Dyn)) { > > + return EFI_UNSUPPORTED; > > + } > > =20 > > // > > // 2. Locate the relocation section from the dynamic section. > > @@ -295,9 +291,6 @@ RelocateElf64Dynamic ( > } > > =20 > > if (RelaOffset =3D=3D MAX_UINT64) { > > - ASSERT (RelaCount =3D=3D 0); > > - ASSERT (RelaEntrySize =3D=3D 0); > > - ASSERT (RelaSize =3D=3D 0); > > // > > // It's fine that a DYN ELF doesn't contain relocation section. > > // > > @@ -308,23 +301,22 @@ RelocateElf64Dynamic ( > // Verify the existence of the relocation section. > > // > > RelShdr =3D GetElf64SectionByRange (ElfCt->FileBase, RelaOffset, Re= laSize); > > - ASSERT (RelShdr !=3D NULL); > > if (RelShdr =3D=3D NULL) { > > return EFI_UNSUPPORTED; > > } > > - ASSERT (RelShdr->sh_type =3D=3D RelaType); > > - ASSERT (RelShdr->sh_entsize =3D=3D RelaEntrySize); > > + if ((RelShdr->sh_type !=3D RelaType) || (RelShdr->sh_entsize !=3D Re= laEntrySize)) { > > + return EFI_UNSUPPORTED; > > + } > > =20 > > // > > // 3. Process the relocation section. > > // > > - ProcessRelocation64 ( > > - (Elf64_Rela *) (ElfCt->FileBase + RelShdr->sh_offset), > > - RelShdr->sh_size, RelShdr->sh_entsize, RelShdr->sh_type, > > - (UINTN) ElfCt->ImageAddress - (UINTN) ElfCt->PreferredImageAddress= , > > - TRUE > > - ); > > - return EFI_SUCCESS; > > + return ProcessRelocation64 ( > > + (Elf64_Rela *) (ElfCt->FileBase + RelShdr->sh_offset), > > + RelShdr->sh_size, RelShdr->sh_entsize, RelShdr->sh_type, > > + (UINTN) ElfCt->ImageAddress - (UINTN) ElfCt->PreferredImage= Address, > > + TRUE > > + ); > > } > > =20 > > /** > > @@ -340,7 +332,6 @@ RelocateElf64Sections ( > IN ELF_IMAGE_CONTEXT *ElfCt > > ) > > { > > - EFI_STATUS Status; > > Elf64_Ehdr *Ehdr; > > Elf64_Shdr *RelShdr; > > Elf64_Shdr *Shdr; > > @@ -360,9 +351,7 @@ RelocateElf64Sections ( > // > > if (Ehdr->e_type =3D=3D ET_DYN) { > > DEBUG ((DEBUG_INFO, "DYN ELF: Relocate using dynamic sections...\= n")); > > - Status =3D RelocateElf64Dynamic (ElfCt); > > - ASSERT_EFI_ERROR (Status); > > - return Status; > > + return RelocateElf64Dynamic (ElfCt); > > } > > =20 > > // > > diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/ElfLib.c b/UefiPay= loadPkg/PayloadLoaderPeim/ElfLib/ElfLib.c > index 531b3486d2..70de81c3ac 100644 > --- a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/ElfLib.c > +++ b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/ElfLib.c > @@ -11,22 +11,32 @@ > /** > > Check if the ELF image is valid. > > =20 > > - @param[in] ImageBase Memory address of an image. > > + @param[in] FileBase Memory address of an image. > > + @param[in] MaxFileSize The maximum file size. > > =20 > > @retval TRUE if valid. > > =20 > > **/ > > BOOLEAN > > IsElfFormat ( > > - IN CONST UINT8 *ImageBase > > + IN CONST UINT8 *FileBase, > > + IN UINTN MaxFileSize > > ) > > { > > Elf32_Ehdr *Elf32Hdr; > > Elf64_Ehdr *Elf64Hdr; > > =20 > > - ASSERT (ImageBase !=3D NULL); > > + ASSERT (FileBase !=3D NULL); > > =20 > > - Elf32Hdr =3D (Elf32_Ehdr *)ImageBase; > > + Elf32Hdr =3D (Elf32_Ehdr *)FileBase; > > + Elf64Hdr =3D (Elf64_Ehdr *)FileBase; > > + > > + // > > + // Make sure MaxFileSize covers e_ident[]. > > + // > > + if (MaxFileSize < sizeof (Elf32Hdr->e_ident)) { > > + return FALSE; > > + } > > =20 > > // > > // Start with correct signature "\7fELF" > > @@ -50,15 +60,13 @@ IsElfFormat ( > // Check 32/64-bit architecture > > // > > if (Elf32Hdr->e_ident[EI_CLASS] =3D=3D ELFCLASS64) { > > - Elf64Hdr =3D (Elf64_Ehdr *)Elf32Hdr; > > - Elf32Hdr =3D NULL; > > - } else if (Elf32Hdr->e_ident[EI_CLASS] =3D=3D ELFCLASS32) { > > - Elf64Hdr =3D NULL; > > - } else { > > - return FALSE; > > - } > > + // > > + // Before accessing fields in Elf64_Ehdr, make sure the MaxFileSiz= e covers the entire header. > > + // > > + if (MaxFileSize < sizeof (Elf64_Ehdr)) { > > + return FALSE; > > + } > > =20 > > - if (Elf64Hdr !=3D NULL) { > > // > > // Support intel architecture only for now > > // > > @@ -79,7 +87,7 @@ IsElfFormat ( > if (Elf64Hdr->e_version !=3D EV_CURRENT) { > > return FALSE; > > } > > - } else { > > + } else if (Elf32Hdr->e_ident[EI_CLASS] =3D=3D ELFCLASS32) { > > // > > // Support intel architecture only for now > > // > > @@ -100,7 +108,10 @@ IsElfFormat ( > if (Elf32Hdr->e_version !=3D EV_CURRENT) { > > return FALSE; > > } > > + } else { > > + return FALSE; > > } > > + > > return TRUE; > > } > > =20 > > @@ -108,6 +119,7 @@ IsElfFormat ( > Calculate a ELF file size. > > =20 > > @param[in] ElfCt ELF image context pointer. > > + @param[in] MaxFileSize The maximum file size. > > @param[out] FileSize Return the file size. > > =20 > > @retval EFI_INVALID_PARAMETER ElfCt or SecPos is NULL. > > @@ -117,12 +129,12 @@ IsElfFormat ( > EFI_STATUS > > CalculateElfFileSize ( > > IN ELF_IMAGE_CONTEXT *ElfCt, > > + IN UINTN MaxFileSize, > > OUT UINTN *FileSize > > ) > > { > > EFI_STATUS Status; > > - UINTN FileSize1; > > - UINTN FileSize2; > > + UINT32 Index; > > Elf32_Ehdr *Elf32Hdr; > > Elf64_Ehdr *Elf64Hdr; > > UINTN Offset; > > @@ -132,24 +144,34 @@ CalculateElfFileSize ( > return EFI_INVALID_PARAMETER; > > } > > =20 > > - // Use last section as end of file > > - Status =3D GetElfSectionPos (ElfCt, ElfCt->ShNum - 1, &Offset, &Size= ); > > - if (EFI_ERROR(Status)) { > > - return EFI_UNSUPPORTED; > > - } > > - FileSize1 =3D Offset + Size; > > - > > - // Use end of section header as end of file > > - FileSize2 =3D 0; > > + // > > + // Optional section headers might exist in the end of file. > > + // > > + *FileSize =3D 0; > > if (ElfCt->EiClass =3D=3D ELFCLASS32) { > > Elf32Hdr =3D (Elf32_Ehdr *)ElfCt->FileBase; > > - FileSize2 =3D Elf32Hdr->e_shoff + Elf32Hdr->e_shentsize * Elf32Hdr= ->e_shnum; > > + *FileSize =3D Elf32Hdr->e_shoff + Elf32Hdr->e_shentsize * Elf32Hdr= ->e_shnum; > > } else if (ElfCt->EiClass =3D=3D ELFCLASS64) { > > Elf64Hdr =3D (Elf64_Ehdr *)ElfCt->FileBase; > > - FileSize2 =3D (UINTN)(Elf64Hdr->e_shoff + Elf64Hdr->e_shentsize * = Elf64Hdr->e_shnum); > > + *FileSize =3D (UINTN)(Elf64Hdr->e_shoff + Elf64Hdr->e_shentsize * = Elf64Hdr->e_shnum); > > } > > =20 > > - *FileSize =3D MAX(FileSize1, FileSize2); > > + // > > + // Get the end of section body. > > + // > > + for (Index =3D 0; Index < ElfCt->ShNum; Index++) { > > + Status =3D GetElfSectionPos (ElfCt, Index, &Offset, &Size); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + if ((Offset >=3D MaxFileSize) || (Size > MaxFileSize - Offset)) { > > + // > > + // Section body is outside of file range. > > + // > > + return EFI_UNSUPPORTED; > > + } > > + *FileSize =3D MAX (*FileSize, Offset + Size); > > + } > > =20 > > return EFI_SUCCESS; > > } > > @@ -213,7 +235,8 @@ GetElfSegmentInfo ( > =20 > > On return, all fields in ElfCt are updated except ImageAddress. > > =20 > > - @param[in] ImageBase Memory address of an image. > > + @param[in] FileBase Memory address of an image. > > + @param[in] MaxFileSize The maximum file size. > > @param[out] ElfCt The EFL image context pointer. > > =20 > > @retval EFI_INVALID_PARAMETER Input parameters are not valid. > > @@ -223,8 +246,9 @@ GetElfSegmentInfo ( > **/ > > EFI_STATUS > > EFIAPI > > -ParseElfImage ( > > - IN VOID *ImageBase, > > +InitializeElfContext ( > > + IN VOID *FileBase, > > + IN UINTN MaxFileSize, > > OUT ELF_IMAGE_CONTEXT *ElfCt > > ) > > { > > @@ -238,30 +262,58 @@ ParseElfImage ( > UINTN End; > > UINTN Base; > > =20 > > - if (ElfCt =3D=3D NULL) { > > - return EFI_INVALID_PARAMETER; > > - } > > + ASSERT (ElfCt !=3D NULL); > > + > > ZeroMem (ElfCt, sizeof(ELF_IMAGE_CONTEXT)); > > =20 > > - if (ImageBase =3D=3D NULL) { > > - return (ElfCt->ParseStatus =3D EFI_INVALID_PARAMETER); > > + if (FileBase =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > > } > > =20 > > - ElfCt->FileBase =3D (UINT8 *)ImageBase; > > - if (!IsElfFormat (ElfCt->FileBase)) { > > - return (ElfCt->ParseStatus =3D EFI_UNSUPPORTED); > > + ElfCt->FileBase =3D (UINT8 *)FileBase; > > + if (!IsElfFormat (ElfCt->FileBase, MaxFileSize)) { > > + return EFI_UNSUPPORTED; > > } > > =20 > > Elf32Hdr =3D (Elf32_Ehdr *)ElfCt->FileBase; > > ElfCt->EiClass =3D Elf32Hdr->e_ident[EI_CLASS]; > > if (ElfCt->EiClass =3D=3D ELFCLASS32) { > > if ((Elf32Hdr->e_type !=3D ET_EXEC) && (Elf32Hdr->e_type !=3D ET_= DYN)) { > > - return (ElfCt->ParseStatus =3D EFI_UNSUPPORTED); > > + return EFI_UNSUPPORTED; > > } > > - Elf32Shdr =3D (Elf32_Shdr *)GetElf32SectionByIndex (ElfCt->FileBas= e, Elf32Hdr->e_shstrndx); > > + > > + if ((Elf32Hdr->e_phoff >=3D MaxFileSize) || ((UINT32) (Elf32Hdr->e= _phentsize * Elf32Hdr->e_phnum) > MaxFileSize - Elf32Hdr->e_phoff)) { > > + // > > + // Program headers are outside of the file range. > > + // > > + return EFI_UNSUPPORTED; > > + } > > + > > + if ((Elf32Hdr->e_shoff >=3D MaxFileSize) || ((UINT32) (Elf32Hdr->e= _shentsize * Elf32Hdr->e_shnum) > MaxFileSize - Elf32Hdr->e_shoff)) { > > + // > > + // Section headers are outside of the file range. > > + // > > + return EFI_UNSUPPORTED; > > + } > > + > > + if (Elf32Hdr->e_entry >=3D MaxFileSize) { > > + // > > + // Entrypoint is outside of the file range. > > + // > > + return EFI_UNSUPPORTED; > > + } > > + > > + Elf32Shdr =3D GetElf32SectionByIndex (ElfCt->FileBase, Elf32Hdr->e= _shstrndx); > > if (Elf32Shdr =3D=3D NULL) { > > - return (ElfCt->ParseStatus =3D EFI_UNSUPPORTED); > > + return EFI_UNSUPPORTED; > > } > > + if ((Elf32Shdr->sh_offset >=3D MaxFileSize) || (Elf32Shdr->sh_size= > MaxFileSize - Elf32Shdr->sh_offset)) { > > + // > > + // String section is outside of the file range. > > + // > > + return EFI_UNSUPPORTED; > > + } > > + > > ElfCt->EntryPoint =3D (UINTN)Elf32Hdr->e_entry; > > ElfCt->ShNum =3D Elf32Hdr->e_shnum; > > ElfCt->PhNum =3D Elf32Hdr->e_phnum; > > @@ -270,12 +322,41 @@ ParseElfImage ( > } else { > > Elf64Hdr =3D (Elf64_Ehdr *)Elf32Hdr; > > if ((Elf64Hdr->e_type !=3D ET_EXEC) && (Elf64Hdr->e_type !=3D ET_= DYN)) { > > - return (ElfCt->ParseStatus =3D EFI_UNSUPPORTED); > > + return EFI_UNSUPPORTED; > > + } > > + > > + if ((Elf64Hdr->e_phoff >=3D MaxFileSize) || ((UINT32) Elf64Hdr->e_= phentsize * Elf64Hdr->e_phnum > MaxFileSize - (UINTN) Elf64Hdr->e_phoff))= { > > + // > > + // Program headers are outside of the file range. > > + // > > + return EFI_UNSUPPORTED; > > + } > > + > > + if ((Elf64Hdr->e_shoff >=3D MaxFileSize) || ((UINT32) Elf64Hdr->e_= shentsize * Elf64Hdr->e_shnum > MaxFileSize - (UINTN) Elf64Hdr->e_shoff))= { > > + // > > + // Section headers are outside of the file range. > > + // > > + return EFI_UNSUPPORTED; > > + } > > + > > + if (Elf64Hdr->e_entry >=3D MaxFileSize) { > > + // > > + // Entrypoint is outside of the file range. > > + // > > + return EFI_UNSUPPORTED; > > } > > - Elf64Shdr =3D (Elf64_Shdr *)GetElf64SectionByIndex (ElfCt->FileBas= e, Elf64Hdr->e_shstrndx); > > + > > + Elf64Shdr =3D GetElf64SectionByIndex (ElfCt->FileBase, Elf64Hdr->e= _shstrndx); > > if (Elf64Shdr =3D=3D NULL) { > > - return (ElfCt->ParseStatus =3D EFI_UNSUPPORTED); > > + return EFI_UNSUPPORTED; > > + } > > + if ((Elf64Shdr->sh_offset >=3D MaxFileSize) || (Elf64Shdr->sh_size= > MaxFileSize - Elf64Shdr->sh_offset)) { > > + // > > + // String section is outside of the file range. > > + // > > + return EFI_UNSUPPORTED; > > } > > + > > ElfCt->EntryPoint =3D (UINTN)Elf64Hdr->e_entry; > > ElfCt->ShNum =3D Elf64Hdr->e_shnum; > > ElfCt->PhNum =3D Elf64Hdr->e_phnum; > > @@ -297,6 +378,13 @@ ParseElfImage ( > continue; > > } > > =20 > > + // > > + // Loadable process segments must have congruent values for p_vadd= r and p_offset, modulo the page size. > > + // > > + if ((SegInfo.MemAddr % EFI_PAGE_SIZE) !=3D (SegInfo.Offset % EFI_P= AGE_SIZE)) { > > + return EFI_UNSUPPORTED; > > + } > > + > > if (SegInfo.MemLen !=3D SegInfo.Length) { > > // > > // Not enough space to execute at current location. > > @@ -317,8 +405,7 @@ ParseElfImage ( > ElfCt->ImageSize =3D End - Base + 1; > > ElfCt->PreferredImageAddress =3D (VOID *) Base; > > =20 > > - CalculateElfFileSize (ElfCt, &ElfCt->FileSize); > > - return (ElfCt->ParseStatus =3D EFI_SUCCESS);; > > + return CalculateElfFileSize (ElfCt, MaxFileSize, &ElfCt->FileSize); > > } > > =20 > > /** > > @@ -348,10 +435,6 @@ LoadElfImage ( > return EFI_INVALID_PARAMETER; > > } > > =20 > > - if (EFI_ERROR (ElfCt->ParseStatus)) { > > - return ElfCt->ParseStatus; > > - } > > - > > if (ElfCt->ImageAddress =3D=3D NULL) { > > return EFI_INVALID_PARAMETER; > > } > > @@ -370,6 +453,8 @@ LoadElfImage ( > /** > > Get a ELF section name from its index. > > =20 > > + ElfCt is returned from InitializeElfContext(). > > + > > @param[in] ElfCt ELF image context pointer. > > @param[in] SectionIndex ELF section index. > > @param[out] SectionName The pointer to the section name. > > @@ -389,25 +474,25 @@ GetElfSectionName ( > Elf32_Shdr *Elf32Shdr; > > Elf64_Shdr *Elf64Shdr; > > CHAR8 *Name; > > + UINTN MaxSize; > > =20 > > if ((ElfCt =3D=3D NULL) || (SectionName =3D=3D NULL)) { > > return EFI_INVALID_PARAMETER; > > } > > =20 > > - if (EFI_ERROR (ElfCt->ParseStatus)) { > > - return ElfCt->ParseStatus; > > - } > > - > > - Name =3D NULL; > > + Name =3D NULL; > > + MaxSize =3D 0; > > if (ElfCt->EiClass =3D=3D ELFCLASS32) { > > Elf32Shdr =3D GetElf32SectionByIndex (ElfCt->FileBase, SectionInd= ex); > > if ((Elf32Shdr !=3D NULL) && (Elf32Shdr->sh_name < ElfCt->ShStrLe= n)) { > > - Name =3D (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf32Shdr= ->sh_name); > > + Name =3D (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf32S= hdr->sh_name); > > + MaxSize =3D ElfCt->ShStrLen - Elf32Shdr->sh_name; > > } > > } else if (ElfCt->EiClass =3D=3D ELFCLASS64) { > > Elf64Shdr =3D GetElf64SectionByIndex (ElfCt->FileBase, SectionInd= ex); > > if ((Elf64Shdr !=3D NULL) && (Elf64Shdr->sh_name < ElfCt->ShStrLe= n)) { > > - Name =3D (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf64Shdr= ->sh_name); > > + Name =3D (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf64S= hdr->sh_name); > > + MaxSize =3D ElfCt->ShStrLen - Elf64Shdr->sh_name; > > } > > } > > =20 > > @@ -415,6 +500,13 @@ GetElfSectionName ( > return EFI_NOT_FOUND; > > } > > =20 > > + if (AsciiStrnLenS (Name, MaxSize) =3D=3D MaxSize) { > > + // > > + // No null terminator is found for the section name. > > + // > > + return EFI_NOT_FOUND; > > + } > > + > > *SectionName =3D Name; > > return EFI_SUCCESS; > > } > > @@ -449,10 +541,6 @@ GetElfSectionPos ( > return EFI_INVALID_PARAMETER; > > } > > =20 > > - if (EFI_ERROR (ElfCt->ParseStatus)) { > > - return ElfCt->ParseStatus; > > - } > > - > > if (ElfCt->EiClass =3D=3D ELFCLASS32) { > > Elf32Shdr =3D GetElf32SectionByIndex (ElfCt->FileBase, Index); > > if (Elf32Shdr !=3D NULL) { > > diff --git a/UefiPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c b/Uef= iPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c > index 44639f9fd2..efedaef1b3 100644 > --- a/UefiPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c > +++ b/UefiPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c > @@ -69,8 +69,10 @@ PeiLoadFileLoadPayload ( > return Status; > > } > > =20 > > - ZeroMem (&Context, sizeof (Context)); > > - Status =3D ParseElfImage (Elf, &Context); > > + // > > + // Trust the ELF image loaded from FV. > > + // > > + Status =3D InitializeElfContext (Elf, MAX_UINTN - (UINTN) Elf, &Co= ntext); > > } while (EFI_ERROR (Status)); > > =20 > > DEBUG (( >