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

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 22 August 2016 at 13:28, Shi, Steven <steven.shi@intel.com> 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; Gao, Liming <liming.gao@intel.com>; Shi,
>> Steven <steven.shi@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> 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 <ard.biesheuvel@linaro.org>
>> ---
>>  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
>