Re: [PATCH] BaseTools/GenFw: ignore dynamic RELA sections

["Gao, Liming" <liming.gao@intel.com>]

Ard:

  I have no comment. Reviewed-by: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>

Thanks
Liming
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Monday, August 22, 2016 7:33 PM
To: Shi, Steven <steven.shi@intel.com>
Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: Re: [PATCH] BaseTools/GenFw: ignore dynamic RELA sections

On 22 August 2016 at 13:28, Shi, Steven wrote:
> Hi Ard,
> I like you path, and it makes the code look more clear. But in fact, old code should already ignore the dynamic RELA sections implicitly.
>
> Like below code, when the GenFw use .rela section info value, which is 0, to get .rela target applying section address in SecShdr, it will point to NULL section [0]. And the next section type checking, which is (*Filter)(SecShdr), will fails and then stop the .rela section relocation process.
>
> Line 685 of BaseTools\Source\C\GenFw\Elf64Convert.c
> //
> // Relocation section found. Now extract section information that the relocations
> // apply to in the ELF data and the new COFF data.
> //
> SecShdr = GetShdrByIndex(RelShdr->sh_info);
> SecOffset = mCoffSectionsOffset[RelShdr->sh_info];
>
> //
> // Only process relocations for the current filter type.
> //
> if (RelShdr->sh_type == SHT_RELA && (*Filter)(SecShdr)) {
> ... ...
> }
>

Indeed. I mentioned this in the commit log:

"""
(and we already ignore it in practice due to the fact that it
points to the NULL section, which has the SHF_ALLOC bit cleared)
"""

But it is important to identify this section explicitly, since it will
have to be present if you add support for GOT based relocations.

>
> But anyway, I still support you add this patch.
>

Thanks,
Ard.


>
>
> Steven Shi
> Intel\SSG\STO\UEFI Firmware
>
> Tel: +86 021-61166522
> iNet: 821-6522
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Monday, August 22, 2016 6:14 PM
>> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Gao, Liming ; Shi,
>> Steven ; Zhu, Yonghong
>> Cc: Ard Biesheuvel
>> Subject: [PATCH] BaseTools/GenFw: ignore dynamic RELA sections
>>
>> When building PIE (ET_DYN) executables, an additional RELA section is
>> emitted (in addition to the per-section .rela.text and .rela.data sections)
>> that is intended to be resolved at runtime by a ET_DYN compatible loader.
>>
>> At the moment, due to the fact that we don't support GOT based relocations,
>> this dynamic RELA section only contains relocations that are redundant,
>> i.e., each R_xxx_RELATIVE relocation it contains duplicates a R_xxx_ABS64
>> relocation appear in .rela.text or .rela.data, and so we can simply ignore
>> this section (and we already ignore it in practice due to the fact that it
>> points to the NULL section, which has the SHF_ALLOC bit cleared)
>>
>> For example,
>>
>> Section Headers:
>> [Nr] Name Type Address Offset
>> Size EntSize Flags Link Info Align
>> [ 0] NULL 0000000000000000 00000000
>> 0000000000000000 0000000000000000 0 0 0
>> [ 1] .text PROGBITS 0000000000000240 000000c0
>> 000000000000427c 0000000000000008 AX 0 0 64
>> [ 2] .rela.text RELA 0000000000000000 00009310
>> 0000000000001bf0 0000000000000018 I 7 1 8
>> [ 3] .data PROGBITS 00000000000044c0 00004340
>> 00000000000046d0 0000000000000000 WA 0 0 64
>> [ 4] .rela.data RELA 0000000000000000 0000af00
>> 0000000000000600 0000000000000018 I 7 3 8
>> [ 5] .rela RELA 0000000000008bc0 00008a10
>> 0000000000000600 0000000000000018 0 0 8
>> [ 6] .shstrtab STRTAB 0000000000000000 0000b500
>> 0000000000000037 0000000000000000 0 0 1
>> [ 7] .symtab SYMTAB 0000000000000000 00009010
>> 0000000000000210 0000000000000018 8 17 8
>> [ 8] .strtab STRTAB 0000000000000000 00009220
>> 00000000000000eb 0000000000000000 0 0 1
>>
>> Relocation section '.rela.data' at offset 0xaf00 contains 64 entries:
>> Offset Info Type Sym. Value Sym. Name + Addend
>> 000000004800 000100000001 R_X86_64_64 0000000000000240 .text +
>> 3f5b
>> 000000004808 000100000001 R_X86_64_64 0000000000000240 .text +
>> 3f63
>> 000000004810 000100000001 R_X86_64_64 0000000000000240 .text +
>> 3f79
>> 000000004818 000100000001 R_X86_64_64 0000000000000240 .text +
>> 3f90
>> 000000004820 000100000001 R_X86_64_64 0000000000000240 .text +
>> 3fa6
>> ...
>>
>> Relocation section '.rela' at offset 0x8a10 contains 64 entries:
>> Offset Info Type Sym. Value Sym. Name + Addend
>> 000000004800 000000000008 R_X86_64_RELATIVE 419b
>> 000000004808 000000000008 R_X86_64_RELATIVE 41a3
>> 000000004810 000000000008 R_X86_64_RELATIVE 41b9
>> 000000004818 000000000008 R_X86_64_RELATIVE 41d0
>> 000000004820 000000000008 R_X86_64_RELATIVE 41e6
>> 000000004828 000000000008 R_X86_64_RELATIVE 41ff
>> ...
>>
>> Note that GOT based relocations result in entries that *only* appear in the
>> dynamic .rela section and not in .rela.text or .rela.data. This means two
>> things for supporting GOT based relocations:
>> - we should check that a dynamic RELA section exists
>> - we should filter out duplicates between .rela and .rela.xxx, to prevent
>> emitting duplicate fixups into the PE/COFF .reloc section.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel
>> ---
>> BaseTools/Source/C/GenFw/Elf64Convert.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
>> b/BaseTools/Source/C/GenFw/Elf64Convert.c
>> index 708c1a1d91a7..acf435712146 100644
>> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
>> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
>> @@ -683,6 +683,20 @@ WriteSections64 (
>> }
>>
>> //
>> + // If this is a ET_DYN (PIE) executable, we will encounter a dynamic
>> SHT_RELA
>> + // section that applies to the entire binary, and which will have its section
>> + // index set to #0 (which is a NULL section with the SHF_ALLOC bit
>> cleared).
>> + //
>> + // In the absence of GOT based relocations (which we currently don't
>> support),
>> + // this RELA section will mostly contain R_xxx_RELATIVE relocations, one
>> for
>> + // every R_xxx_ABS64 relocation appearing in the per-section RELA
>> sections.
>> + // (i.e., .rela.text and .rela.data)
>> + //
>> + if (RelShdr->sh_info == 0) {
>> + continue;
>> + }
>> +
>> + //
>> // Relocation section found. Now extract section information that the
>> relocations
>> // apply to in the ELF data and the new COFF data.
>> //
>> --
>> 2.7.4
>