Reviewed-by: Guo Dong <guo.d...@intel.com>
> -----Original Message----- > From: Ni, Ray <ray...@intel.com> > Sent: Monday, June 28, 2021 11:27 PM > To: devel@edk2.groups.io > Cc: Ma, Maurice <maurice...@intel.com>; Dong, Guo > <guo.d...@intel.com>; You, Benjamin <benjamin....@intel.com> > Subject: [PATCH 1/2] UefiPayloadPkg/PayloadLoader: Fix bug in locating > relocation section > > Per ELF spec, the DT_REL/DT_RELA tag in dynamic section stores the > virtual address of the relocation section. > > But today's code logic treats it as the section offset and finds > the relocation section whose offset equals to DT_REL/DT_RELA. > > The logic can work when the section offset equals to the section > virtual address. But when the ELF is generated from the link script > that reserves a sizeof(pe_header) in the file beginning, the section > offset doesn't equal to section virtual address. Such logic can > not find the relocation section. > > The patch fixes this bug. > > Signed-off-by: Ray Ni <ray...@intel.com> > Cc: Maurice Ma <maurice...@intel.com> > Cc: Guo Dong <guo.d...@intel.com> > Cc: Benjamin You <benjamin....@intel.com> > --- > .../PayloadLoaderPeim/ElfLib/Elf32Lib.c | 22 +++++++++++++------ > .../PayloadLoaderPeim/ElfLib/Elf64Lib.c | 22 +++++++++++++------ > 2 files changed, 30 insertions(+), 14 deletions(-) > > diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c > b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c > index 3fa100ce4a..dd27d3ce59 100644 > --- a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c > +++ b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c > @@ -206,7 +206,7 @@ RelocateElf32Dynamic ( > Elf32_Shdr *DynShdr; > > Elf32_Shdr *RelShdr; > > Elf32_Dyn *Dyn; > > - UINT32 RelaOffset; > > + UINT32 RelaAddress; > > UINT32 RelaCount; > > UINT32 RelaSize; > > UINT32 RelaEntrySize; > > @@ -246,7 +246,7 @@ RelocateElf32Dynamic ( > // > > // 2. Locate the relocation section from the dynamic section. > > // > > - RelaOffset = MAX_UINT32; > > + RelaAddress = MAX_UINT32; > > RelaSize = 0; > > RelaCount = 0; > > RelaEntrySize = 0; > > @@ -265,8 +265,8 @@ RelocateElf32Dynamic ( > // based on the original file value and the memory base address. > > // For consistency, files do not contain relocation entries to > ``correct'' > addresses in the dynamic structure. > > // > > - RelaOffset = Dyn->d_un.d_ptr - (UINT32) (UINTN) ElfCt- > >PreferredImageAddress; > > - RelaType = (Dyn->d_tag == DT_RELA) ? SHT_RELA: SHT_REL; > > + RelaAddress = Dyn->d_un.d_ptr; > > + RelaType = (Dyn->d_tag == DT_RELA) ? SHT_RELA: SHT_REL; > > break; > > case DT_RELACOUNT: > > case DT_RELCOUNT: > > @@ -285,7 +285,7 @@ RelocateElf32Dynamic ( > } > > } > > > > - if (RelaOffset == MAX_UINT64) { > > + if (RelaAddress == MAX_UINT64) { > > ASSERT (RelaCount == 0); > > ASSERT (RelaEntrySize == 0); > > ASSERT (RelaSize == 0); > > @@ -298,8 +298,16 @@ RelocateElf32Dynamic ( > // > > // Verify the existence of the relocation section. > > // > > - RelShdr = GetElf32SectionByRange (ElfCt->FileBase, RelaOffset, RelaSize); > > - ASSERT (RelShdr != NULL); > > + RelShdr = NULL; > > + for (Index = 0; Index < ElfCt->ShNum; Index++) { > > + RelShdr = GetElf32SectionByIndex (ElfCt->FileBase, Index); > > + ASSERT (RelShdr != NULL); > > + if ((RelShdr->sh_addr == RelaAddress) && (RelShdr->sh_size == RelaSize)) > { > > + break; > > + } > > + RelShdr = NULL; > > + } > > + > > if (RelShdr == NULL) { > > return EFI_UNSUPPORTED; > > } > > diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c > b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c > index e364807007..3f4f12903c 100644 > --- a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c > +++ b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c > @@ -215,7 +215,7 @@ RelocateElf64Dynamic ( > Elf64_Shdr *DynShdr; > > Elf64_Shdr *RelShdr; > > Elf64_Dyn *Dyn; > > - UINT64 RelaOffset; > > + UINT64 RelaAddress; > > UINT64 RelaCount; > > UINT64 RelaSize; > > UINT64 RelaEntrySize; > > @@ -255,7 +255,7 @@ RelocateElf64Dynamic ( > // > > // 2. Locate the relocation section from the dynamic section. > > // > > - RelaOffset = MAX_UINT64; > > + RelaAddress = MAX_UINT64; > > RelaSize = 0; > > RelaCount = 0; > > RelaEntrySize = 0; > > @@ -274,8 +274,8 @@ RelocateElf64Dynamic ( > // based on the original file value and the memory base address. > > // For consistency, files do not contain relocation entries to > ``correct'' > addresses in the dynamic structure. > > // > > - RelaOffset = Dyn->d_un.d_ptr - (UINTN) ElfCt->PreferredImageAddress; > > - RelaType = (Dyn->d_tag == DT_RELA) ? SHT_RELA: SHT_REL; > > + RelaAddress = Dyn->d_un.d_ptr; > > + RelaType = (Dyn->d_tag == DT_RELA) ? SHT_RELA: SHT_REL; > > break; > > case DT_RELACOUNT: > > case DT_RELCOUNT: > > @@ -294,7 +294,7 @@ RelocateElf64Dynamic ( > } > > } > > > > - if (RelaOffset == MAX_UINT64) { > > + if (RelaAddress == MAX_UINT64) { > > ASSERT (RelaCount == 0); > > ASSERT (RelaEntrySize == 0); > > ASSERT (RelaSize == 0); > > @@ -307,8 +307,16 @@ RelocateElf64Dynamic ( > // > > // Verify the existence of the relocation section. > > // > > - RelShdr = GetElf64SectionByRange (ElfCt->FileBase, RelaOffset, RelaSize); > > - ASSERT (RelShdr != NULL); > > + RelShdr = NULL; > > + for (Index = 0; Index < ElfCt->ShNum; Index++) { > > + RelShdr = GetElf64SectionByIndex (ElfCt->FileBase, Index); > > + ASSERT (RelShdr != NULL); > > + if ((RelShdr->sh_addr == RelaAddress) && (RelShdr->sh_size == RelaSize)) > { > > + break; > > + } > > + RelShdr = NULL; > > + } > > + > > if (RelShdr == NULL) { > > return EFI_UNSUPPORTED; > > } > > -- > 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#77311): https://edk2.groups.io/g/devel/message/77311 Mute This Topic: https://groups.io/mt/83863241/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-