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]
-=-=-=-=-=-=-=-=-=-=-=-
