[edk2-devel] [PATCH] UefiPayloadPkg/PayloadLoader: Add more checks to verify ELF images
Marvin Häuser
mhaeuser at posteo.de
Mon Jun 28 11:03:00 UTC 2021
Hey Ray,
Sorry for not having properly checked yet, I definitely plan to still.
However, I probably won't till a pointer alignment macro lands (I plan
to submit a bunch of things, including this, within the next two weeks).
Once it has been merged, I think this patch can be improved with
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 <ray.ni at intel.com>
> Cc: Marvin Häuser <mhaeuser at posteo.de>
> Cc: Guo Dong <guo.dong at intel.com>
> Cc: Benjamin You <benjamin.you at intel.com>
> ---
> 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
>
>
>
> typedef struct {
>
> - RETURN_STATUS ParseStatus; ///< Return the status after ParseElfImage().
>
> UINT8 *FileBase; ///< The source location in memory.
>
> UINTN FileSize; ///< The size including sections 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.
>
>
>
> - @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.
>
>
>
> @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
>
> );
>
>
>
> diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c b/UefiPayloadPkg/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;
>
>
>
> for ( Index = 0
>
> - ; RelaEntrySize * Index < RelaSize
>
> + ; Index < RelaSize / RelaEntrySize
>
> ; Index++, Rela = ELF_NEXT_ENTRY (Elf32_Rela, Rela, RelaEntrySize)
>
> ) {
>
> //
>
> @@ -137,7 +137,6 @@ ProcessRelocation32 (
> // Dynamic section doesn't contain entries of this type.
>
> //
>
> DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", Type));
>
> - ASSERT (FALSE);
>
> } else {
>
> *Ptr += (UINT32) Delta;
>
> }
>
> @@ -164,7 +163,6 @@ ProcessRelocation32 (
> // Calculation: B + A
>
> //
>
> if (RelaType == SHT_RELA) {
>
> - ASSERT (*Ptr == 0);
>
> *Ptr = (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", Type));
>
> - ASSERT (FALSE);
>
> }
>
> break;
>
>
>
> @@ -236,12 +233,12 @@ RelocateElf32Dynamic (
> //
>
> // It's abnormal a DYN ELF doesn't contain a dynamic section.
>
> //
>
> - ASSERT (DynShdr != NULL);
>
> if (DynShdr == NULL) {
>
> return EFI_UNSUPPORTED;
>
> }
>
> - ASSERT (DynShdr->sh_type == SHT_DYNAMIC);
>
> - ASSERT (DynShdr->sh_entsize >= sizeof (*Dyn));
>
> + if ((DynShdr->sh_type != SHT_DYNAMIC) || DynShdr->sh_entsize < sizeof (*Dyn)) {
>
> + return EFI_UNSUPPORTED;
>
> + }
>
>
>
> //
>
> // 2. Locate the relocation section from the dynamic section.
>
> @@ -286,9 +283,6 @@ RelocateElf32Dynamic (
> }
>
>
>
> if (RelaOffset == MAX_UINT64) {
>
> - ASSERT (RelaCount == 0);
>
> - ASSERT (RelaEntrySize == 0);
>
> - ASSERT (RelaSize == 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 = GetElf32SectionByRange (ElfCt->FileBase, RelaOffset, RelaSize);
>
> - ASSERT (RelShdr != NULL);
>
> if (RelShdr == NULL) {
>
> return EFI_UNSUPPORTED;
>
> }
>
> - ASSERT (RelShdr->sh_type == RelaType);
>
> - ASSERT (RelShdr->sh_entsize == RelaEntrySize);
>
> + if ((RelShdr->sh_type != RelaType) || (RelShdr->sh_entsize != RelaEntrySize)) {
>
> + return EFI_UNSUPPORTED;
>
> + }
>
>
>
> //
>
> // 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->PreferredImageAddress,
>
> + TRUE
>
> + );
>
> }
>
>
>
> /**
>
> @@ -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 == ET_DYN) {
>
> DEBUG ((DEBUG_INFO, "DYN ELF: Relocate using dynamic sections...\n"));
>
> - Status = RelocateElf32Dynamic (ElfCt);
>
> - ASSERT_EFI_ERROR (Status);
>
> - return Status;
>
> + return RelocateElf32Dynamic (ElfCt);
>
> }
>
>
>
> //
>
> diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c b/UefiPayloadPkg/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;
>
>
>
> for ( Index = 0
>
> - ; MultU64x64 (RelaEntrySize, Index) < RelaSize
>
> + ; Index < DivU64x64Remainder (RelaSize, RelaEntrySize, NULL)
>
> ; Index++, Rela = ELF_NEXT_ENTRY (Elf64_Rela, Rela, RelaEntrySize)
>
> ) {
>
> //
>
> @@ -138,7 +138,6 @@ ProcessRelocation64 (
> // Dynamic section doesn't contain entries of this type.
>
> //
>
> DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", Type));
>
> - ASSERT (FALSE);
>
> } else {
>
> *Ptr += Delta;
>
> }
>
> @@ -149,7 +148,6 @@ ProcessRelocation64 (
> // Dynamic section doesn't contain entries of this type.
>
> //
>
> DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", Type));
>
> - ASSERT (FALSE);
>
> break;
>
>
>
> case R_X86_64_RELATIVE:
>
> @@ -173,7 +171,6 @@ ProcessRelocation64 (
> // Calculation: B + A
>
> //
>
> if (RelaType == SHT_RELA) {
>
> - ASSERT (*Ptr == 0);
>
> *Ptr = 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", Type));
>
> - ASSERT (FALSE);
>
> }
>
> break;
>
>
>
> @@ -245,12 +241,12 @@ RelocateElf64Dynamic (
> //
>
> // It's abnormal a DYN ELF doesn't contain a dynamic section.
>
> //
>
> - ASSERT (DynShdr != NULL);
>
> if (DynShdr == NULL) {
>
> return EFI_UNSUPPORTED;
>
> }
>
> - ASSERT (DynShdr->sh_type == SHT_DYNAMIC);
>
> - ASSERT (DynShdr->sh_entsize >= sizeof (*Dyn));
>
> + if ((DynShdr->sh_type != SHT_DYNAMIC) || DynShdr->sh_entsize < sizeof (*Dyn)) {
>
> + return EFI_UNSUPPORTED;
>
> + }
>
>
>
> //
>
> // 2. Locate the relocation section from the dynamic section.
>
> @@ -295,9 +291,6 @@ RelocateElf64Dynamic (
> }
>
>
>
> if (RelaOffset == MAX_UINT64) {
>
> - ASSERT (RelaCount == 0);
>
> - ASSERT (RelaEntrySize == 0);
>
> - ASSERT (RelaSize == 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 = GetElf64SectionByRange (ElfCt->FileBase, RelaOffset, RelaSize);
>
> - ASSERT (RelShdr != NULL);
>
> if (RelShdr == NULL) {
>
> return EFI_UNSUPPORTED;
>
> }
>
> - ASSERT (RelShdr->sh_type == RelaType);
>
> - ASSERT (RelShdr->sh_entsize == RelaEntrySize);
>
> + if ((RelShdr->sh_type != RelaType) || (RelShdr->sh_entsize != RelaEntrySize)) {
>
> + return EFI_UNSUPPORTED;
>
> + }
>
>
>
> //
>
> // 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->PreferredImageAddress,
>
> + TRUE
>
> + );
>
> }
>
>
>
> /**
>
> @@ -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 == ET_DYN) {
>
> DEBUG ((DEBUG_INFO, "DYN ELF: Relocate using dynamic sections...\n"));
>
> - Status = RelocateElf64Dynamic (ElfCt);
>
> - ASSERT_EFI_ERROR (Status);
>
> - return Status;
>
> + return RelocateElf64Dynamic (ElfCt);
>
> }
>
>
>
> //
>
> diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/ElfLib.c b/UefiPayloadPkg/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.
>
>
>
> - @param[in] ImageBase Memory address of an image.
>
> + @param[in] FileBase Memory address of an image.
>
> + @param[in] MaxFileSize The maximum file size.
>
>
>
> @retval TRUE if valid.
>
>
>
> **/
>
> BOOLEAN
>
> IsElfFormat (
>
> - IN CONST UINT8 *ImageBase
>
> + IN CONST UINT8 *FileBase,
>
> + IN UINTN MaxFileSize
>
> )
>
> {
>
> Elf32_Ehdr *Elf32Hdr;
>
> Elf64_Ehdr *Elf64Hdr;
>
>
>
> - ASSERT (ImageBase != NULL);
>
> + ASSERT (FileBase != NULL);
>
>
>
> - Elf32Hdr = (Elf32_Ehdr *)ImageBase;
>
> + Elf32Hdr = (Elf32_Ehdr *)FileBase;
>
> + Elf64Hdr = (Elf64_Ehdr *)FileBase;
>
> +
>
> + //
>
> + // Make sure MaxFileSize covers e_ident[].
>
> + //
>
> + if (MaxFileSize < sizeof (Elf32Hdr->e_ident)) {
>
> + return FALSE;
>
> + }
>
>
>
> //
>
> // Start with correct signature "\7fELF"
>
> @@ -50,15 +60,13 @@ IsElfFormat (
> // Check 32/64-bit architecture
>
> //
>
> if (Elf32Hdr->e_ident[EI_CLASS] == ELFCLASS64) {
>
> - Elf64Hdr = (Elf64_Ehdr *)Elf32Hdr;
>
> - Elf32Hdr = NULL;
>
> - } else if (Elf32Hdr->e_ident[EI_CLASS] == ELFCLASS32) {
>
> - Elf64Hdr = NULL;
>
> - } else {
>
> - return FALSE;
>
> - }
>
> + //
>
> + // Before accessing fields in Elf64_Ehdr, make sure the MaxFileSize covers the entire header.
>
> + //
>
> + if (MaxFileSize < sizeof (Elf64_Ehdr)) {
>
> + return FALSE;
>
> + }
>
>
>
> - if (Elf64Hdr != NULL) {
>
> //
>
> // Support intel architecture only for now
>
> //
>
> @@ -79,7 +87,7 @@ IsElfFormat (
> if (Elf64Hdr->e_version != EV_CURRENT) {
>
> return FALSE;
>
> }
>
> - } else {
>
> + } else if (Elf32Hdr->e_ident[EI_CLASS] == ELFCLASS32) {
>
> //
>
> // Support intel architecture only for now
>
> //
>
> @@ -100,7 +108,10 @@ IsElfFormat (
> if (Elf32Hdr->e_version != EV_CURRENT) {
>
> return FALSE;
>
> }
>
> + } else {
>
> + return FALSE;
>
> }
>
> +
>
> return TRUE;
>
> }
>
>
>
> @@ -108,6 +119,7 @@ IsElfFormat (
> Calculate a ELF file size.
>
>
>
> @param[in] ElfCt ELF image context pointer.
>
> + @param[in] MaxFileSize The maximum file size.
>
> @param[out] FileSize Return the file size.
>
>
>
> @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;
>
> }
>
>
>
> - // Use last section as end of file
>
> - Status = GetElfSectionPos (ElfCt, ElfCt->ShNum - 1, &Offset, &Size);
>
> - if (EFI_ERROR(Status)) {
>
> - return EFI_UNSUPPORTED;
>
> - }
>
> - FileSize1 = Offset + Size;
>
> -
>
> - // Use end of section header as end of file
>
> - FileSize2 = 0;
>
> + //
>
> + // Optional section headers might exist in the end of file.
>
> + //
>
> + *FileSize = 0;
>
> if (ElfCt->EiClass == ELFCLASS32) {
>
> Elf32Hdr = (Elf32_Ehdr *)ElfCt->FileBase;
>
> - FileSize2 = Elf32Hdr->e_shoff + Elf32Hdr->e_shentsize * Elf32Hdr->e_shnum;
>
> + *FileSize = Elf32Hdr->e_shoff + Elf32Hdr->e_shentsize * Elf32Hdr->e_shnum;
>
> } else if (ElfCt->EiClass == ELFCLASS64) {
>
> Elf64Hdr = (Elf64_Ehdr *)ElfCt->FileBase;
>
> - FileSize2 = (UINTN)(Elf64Hdr->e_shoff + Elf64Hdr->e_shentsize * Elf64Hdr->e_shnum);
>
> + *FileSize = (UINTN)(Elf64Hdr->e_shoff + Elf64Hdr->e_shentsize * Elf64Hdr->e_shnum);
>
> }
>
>
>
> - *FileSize = MAX(FileSize1, FileSize2);
>
> + //
>
> + // Get the end of section body.
>
> + //
>
> + for (Index = 0; Index < ElfCt->ShNum; Index++) {
>
> + Status = GetElfSectionPos (ElfCt, Index, &Offset, &Size);
>
> + if (EFI_ERROR (Status)) {
>
> + return Status;
>
> + }
>
> + if ((Offset >= MaxFileSize) || (Size > MaxFileSize - Offset)) {
>
> + //
>
> + // Section body is outside of file range.
>
> + //
>
> + return EFI_UNSUPPORTED;
>
> + }
>
> + *FileSize = MAX (*FileSize, Offset + Size);
>
> + }
>
>
>
> return EFI_SUCCESS;
>
> }
>
> @@ -213,7 +235,8 @@ GetElfSegmentInfo (
>
>
> On return, all fields in ElfCt are updated except ImageAddress.
>
>
>
> - @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.
>
>
>
> @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;
>
>
>
> - if (ElfCt == NULL) {
>
> - return EFI_INVALID_PARAMETER;
>
> - }
>
> + ASSERT (ElfCt != NULL);
>
> +
>
> ZeroMem (ElfCt, sizeof(ELF_IMAGE_CONTEXT));
>
>
>
> - if (ImageBase == NULL) {
>
> - return (ElfCt->ParseStatus = EFI_INVALID_PARAMETER);
>
> + if (FileBase == NULL) {
>
> + return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - ElfCt->FileBase = (UINT8 *)ImageBase;
>
> - if (!IsElfFormat (ElfCt->FileBase)) {
>
> - return (ElfCt->ParseStatus = EFI_UNSUPPORTED);
>
> + ElfCt->FileBase = (UINT8 *)FileBase;
>
> + if (!IsElfFormat (ElfCt->FileBase, MaxFileSize)) {
>
> + return EFI_UNSUPPORTED;
>
> }
>
>
>
> Elf32Hdr = (Elf32_Ehdr *)ElfCt->FileBase;
>
> ElfCt->EiClass = Elf32Hdr->e_ident[EI_CLASS];
>
> if (ElfCt->EiClass == ELFCLASS32) {
>
> if ((Elf32Hdr->e_type != ET_EXEC) && (Elf32Hdr->e_type != ET_DYN)) {
>
> - return (ElfCt->ParseStatus = EFI_UNSUPPORTED);
>
> + return EFI_UNSUPPORTED;
>
> }
>
> - Elf32Shdr = (Elf32_Shdr *)GetElf32SectionByIndex (ElfCt->FileBase, Elf32Hdr->e_shstrndx);
>
> +
>
> + if ((Elf32Hdr->e_phoff >= 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 >= 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 >= MaxFileSize) {
>
> + //
>
> + // Entrypoint is outside of the file range.
>
> + //
>
> + return EFI_UNSUPPORTED;
>
> + }
>
> +
>
> + Elf32Shdr = GetElf32SectionByIndex (ElfCt->FileBase, Elf32Hdr->e_shstrndx);
>
> if (Elf32Shdr == NULL) {
>
> - return (ElfCt->ParseStatus = EFI_UNSUPPORTED);
>
> + return EFI_UNSUPPORTED;
>
> }
>
> + if ((Elf32Shdr->sh_offset >= MaxFileSize) || (Elf32Shdr->sh_size > MaxFileSize - Elf32Shdr->sh_offset)) {
>
> + //
>
> + // String section is outside of the file range.
>
> + //
>
> + return EFI_UNSUPPORTED;
>
> + }
>
> +
>
> ElfCt->EntryPoint = (UINTN)Elf32Hdr->e_entry;
>
> ElfCt->ShNum = Elf32Hdr->e_shnum;
>
> ElfCt->PhNum = Elf32Hdr->e_phnum;
>
> @@ -270,12 +322,41 @@ ParseElfImage (
> } else {
>
> Elf64Hdr = (Elf64_Ehdr *)Elf32Hdr;
>
> if ((Elf64Hdr->e_type != ET_EXEC) && (Elf64Hdr->e_type != ET_DYN)) {
>
> - return (ElfCt->ParseStatus = EFI_UNSUPPORTED);
>
> + return EFI_UNSUPPORTED;
>
> + }
>
> +
>
> + if ((Elf64Hdr->e_phoff >= 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 >= 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 >= MaxFileSize) {
>
> + //
>
> + // Entrypoint is outside of the file range.
>
> + //
>
> + return EFI_UNSUPPORTED;
>
> }
>
> - Elf64Shdr = (Elf64_Shdr *)GetElf64SectionByIndex (ElfCt->FileBase, Elf64Hdr->e_shstrndx);
>
> +
>
> + Elf64Shdr = GetElf64SectionByIndex (ElfCt->FileBase, Elf64Hdr->e_shstrndx);
>
> if (Elf64Shdr == NULL) {
>
> - return (ElfCt->ParseStatus = EFI_UNSUPPORTED);
>
> + return EFI_UNSUPPORTED;
>
> + }
>
> + if ((Elf64Shdr->sh_offset >= MaxFileSize) || (Elf64Shdr->sh_size > MaxFileSize - Elf64Shdr->sh_offset)) {
>
> + //
>
> + // String section is outside of the file range.
>
> + //
>
> + return EFI_UNSUPPORTED;
>
> }
>
> +
>
> ElfCt->EntryPoint = (UINTN)Elf64Hdr->e_entry;
>
> ElfCt->ShNum = Elf64Hdr->e_shnum;
>
> ElfCt->PhNum = Elf64Hdr->e_phnum;
>
> @@ -297,6 +378,13 @@ ParseElfImage (
> continue;
>
> }
>
>
>
> + //
>
> + // Loadable process segments must have congruent values for p_vaddr and p_offset, modulo the page size.
>
> + //
>
> + if ((SegInfo.MemAddr % EFI_PAGE_SIZE) != (SegInfo.Offset % EFI_PAGE_SIZE)) {
>
> + return EFI_UNSUPPORTED;
>
> + }
>
> +
>
> if (SegInfo.MemLen != SegInfo.Length) {
>
> //
>
> // Not enough space to execute at current location.
>
> @@ -317,8 +405,7 @@ ParseElfImage (
> ElfCt->ImageSize = End - Base + 1;
>
> ElfCt->PreferredImageAddress = (VOID *) Base;
>
>
>
> - CalculateElfFileSize (ElfCt, &ElfCt->FileSize);
>
> - return (ElfCt->ParseStatus = EFI_SUCCESS);;
>
> + return CalculateElfFileSize (ElfCt, MaxFileSize, &ElfCt->FileSize);
>
> }
>
>
>
> /**
>
> @@ -348,10 +435,6 @@ LoadElfImage (
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if (EFI_ERROR (ElfCt->ParseStatus)) {
>
> - return ElfCt->ParseStatus;
>
> - }
>
> -
>
> if (ElfCt->ImageAddress == NULL) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
> @@ -370,6 +453,8 @@ LoadElfImage (
> /**
>
> Get a ELF section name from its index.
>
>
>
> + 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;
>
>
>
> if ((ElfCt == NULL) || (SectionName == NULL)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if (EFI_ERROR (ElfCt->ParseStatus)) {
>
> - return ElfCt->ParseStatus;
>
> - }
>
> -
>
> - Name = NULL;
>
> + Name = NULL;
>
> + MaxSize = 0;
>
> if (ElfCt->EiClass == ELFCLASS32) {
>
> Elf32Shdr = GetElf32SectionByIndex (ElfCt->FileBase, SectionIndex);
>
> if ((Elf32Shdr != NULL) && (Elf32Shdr->sh_name < ElfCt->ShStrLen)) {
>
> - Name = (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf32Shdr->sh_name);
>
> + Name = (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf32Shdr->sh_name);
>
> + MaxSize = ElfCt->ShStrLen - Elf32Shdr->sh_name;
>
> }
>
> } else if (ElfCt->EiClass == ELFCLASS64) {
>
> Elf64Shdr = GetElf64SectionByIndex (ElfCt->FileBase, SectionIndex);
>
> if ((Elf64Shdr != NULL) && (Elf64Shdr->sh_name < ElfCt->ShStrLen)) {
>
> - Name = (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf64Shdr->sh_name);
>
> + Name = (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf64Shdr->sh_name);
>
> + MaxSize = ElfCt->ShStrLen - Elf64Shdr->sh_name;
>
> }
>
> }
>
>
>
> @@ -415,6 +500,13 @@ GetElfSectionName (
> return EFI_NOT_FOUND;
>
> }
>
>
>
> + if (AsciiStrnLenS (Name, MaxSize) == MaxSize) {
>
> + //
>
> + // No null terminator is found for the section name.
>
> + //
>
> + return EFI_NOT_FOUND;
>
> + }
>
> +
>
> *SectionName = Name;
>
> return EFI_SUCCESS;
>
> }
>
> @@ -449,10 +541,6 @@ GetElfSectionPos (
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if (EFI_ERROR (ElfCt->ParseStatus)) {
>
> - return ElfCt->ParseStatus;
>
> - }
>
> -
>
> if (ElfCt->EiClass == ELFCLASS32) {
>
> Elf32Shdr = GetElf32SectionByIndex (ElfCt->FileBase, Index);
>
> if (Elf32Shdr != NULL) {
>
> diff --git a/UefiPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c b/UefiPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c
> index 44639f9fd2..efedaef1b3 100644
> --- a/UefiPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c
> +++ b/UefiPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c
> @@ -69,8 +69,10 @@ PeiLoadFileLoadPayload (
> return Status;
>
> }
>
>
>
> - ZeroMem (&Context, sizeof (Context));
>
> - Status = ParseElfImage (Elf, &Context);
>
> + //
>
> + // Trust the ELF image loaded from FV.
>
> + //
>
> + Status = InitializeElfContext (Elf, MAX_UINTN - (UINTN) Elf, &Context);
>
> } while (EFI_ERROR (Status));
>
>
>
> DEBUG ((
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77178): https://edk2.groups.io/g/devel/message/77178
Mute This Topic: https://groups.io/mt/83485388/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list