From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22d.google.com (mail-it0-x22d.google.com [IPv6:2607:f8b0:4001:c0b::22d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id DBA3F1A1DF4 for ; Mon, 22 Aug 2016 04:32:34 -0700 (PDT) Received: by mail-it0-x22d.google.com with SMTP id f6so79315929ith.0 for ; Mon, 22 Aug 2016 04:32:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=eKfNtzrdaYozsRCSSW77Zo3bodbaNpFpIpk8UkiBQNs=; b=AClAN9blsLutu9m3j4GfUOFWFEpj4eMEneD6k6TtQuygSOg1KiRoYALUCvz4fzKuJ4 cHZiPQEOZpsmG8UpVcmqU7wim4IYov6c60zTAFsESwczo55LEa1SQJeI0mLp3320E6Mg YsUtMrYNogfBUZNJrZJ+qYTlTctU4x2pZVD1Y= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=eKfNtzrdaYozsRCSSW77Zo3bodbaNpFpIpk8UkiBQNs=; b=IgGgP72L3tDsQS2/CPiw+RJUBjjYII2wN6YIHSERIFdXxhBlvjsGchkL0eWszq6S78 GyN5ushx7Jjtb70J/g3WBvUwJ9G2Dxbe4mv3XQdcML0o+3iM9sY32nJJ581YcOK912Eh uh9Apa5URAjMKHCOULXIkgt8hWTEDhLt4Thb1hU+MUzrAtpBeKry7ujzWYRETKtdsm11 Ld9DBaVTXnDuwZhIE+OHDHdumo+cdshUUOx+JaI45AAhYSROodNpIqt9a2EmyPOFjy7u PSS6OpnCC/GvRqr3iNCg/tyVkFdjXyAyyf+h70VDVZCVbsEmRw+0LOW+O73kJ76V4hvc ZDsg== X-Gm-Message-State: AEkooutFH7Mm4ibc3GJKLUeZ0HLbQR4murSeOoSdlg3wJAsmbEYiC9h7lNU6XvsyPq2tA6hk3XvZH6zrMILtqYaN X-Received: by 10.36.25.144 with SMTP id b138mr19880683itb.29.1471865553865; Mon, 22 Aug 2016 04:32:33 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Mon, 22 Aug 2016 04:32:33 -0700 (PDT) In-Reply-To: <06C8AB66E78EE34A949939824ABE2B31033908B1@shsmsx102.ccr.corp.intel.com> References: <1471860861-32600-1-git-send-email-ard.biesheuvel@linaro.org> <06C8AB66E78EE34A949939824ABE2B31033908B1@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Mon, 22 Aug 2016 13:32:33 +0200 Message-ID: To: "Shi, Steven" Cc: "edk2-devel@lists.01.org" , "Gao, Liming" , "Zhu, Yonghong" Subject: Re: [PATCH] BaseTools/GenFw: ignore dynamic RELA sections X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Aug 2016 11:32:35 -0000 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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)(Se= cShdr), 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 th= e relocations > // apply to in the ELF data and the new COFF data. > // > SecShdr =3D GetShdrByIndex(RelShdr->sh_info); > SecOffset =3D mCoffSectionsOffset[RelShdr->sh_info]; > > // > // Only process relocations for the current filter type. > // > if (RelShdr->sh_type =3D=3D 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 ; 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 sectio= ns) >> 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 relocatio= ns, >> this dynamic RELA section only contains relocations that are redundant, >> i.e., each R_xxx_RELATIVE relocation it contains duplicates a R_xxx_ABS6= 4 >> relocation appear in .rela.text or .rela.data, and so we can simply igno= re >> 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. Nam= e + 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. Nam= e + 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 tw= o >> 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 preven= t >> 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 dynami= c >> SHT_RELA >> + // section that applies to the entire binary, and which will have i= ts 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 =3D=3D 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 >