From: "Ni, Ray" <ray.ni@intel.com>
To: devel@edk2.groups.io
Cc: "Marvin Häuser" <mhaeuser@posteo.de>,
"Guo Dong" <guo.dong@intel.com>,
"Benjamin You" <benjamin.you@intel.com>
Subject: [PATCH] UefiPayloadPkg/PayloadLoader: Add more checks to verify ELF images
Date: Sat, 12 Jun 2021 14:03:04 +0800 [thread overview]
Message-ID: <20210612060304.43-1-ray.ni@intel.com> (raw)
More checks are added to verify ELF image.
ParseElfImage() is changed to InitializeElfContext()
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Benjamin You <benjamin.you@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 ((
--
2.31.1.windows.1
next reply other threads:[~2021-06-12 6:03 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-12 6:03 Ni, Ray [this message]
2021-06-28 11:03 ` [edk2-devel] [PATCH] UefiPayloadPkg/PayloadLoader: Add more checks to verify ELF images Marvin Häuser
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210612060304.43-1-ray.ni@intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox