[lld] r291524 - ELF: Reserve space for copy relocations of read-only symbols in relro.
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 12 06:43:06 PST 2017
This changes causes crashes on FreeBSD. Looks like the freebsd dynamic
linker is setting the area read only too early.
Ed, any idea how to fix this?
Cheers,
Rafael
Peter Collingbourne via llvm-commits <llvm-commits at lists.llvm.org>
writes:
> Author: pcc
> Date: Mon Jan 9 19:21:50 2017
> New Revision: 291524
>
> URL: http://llvm.org/viewvc/llvm-project?rev=291524&view=rev
> Log:
> ELF: Reserve space for copy relocations of read-only symbols in relro.
>
> When reserving copy relocation space for a shared symbol, scan the DSO's
> program headers to see if the symbol is in a read-only segment. If so,
> reserve space for that symbol in a new synthetic section named .bss.rel.ro
> which will be covered by the relro program header.
>
> This fixes the security issue disclosed on the binutils mailing list at:
> https://sourceware.org/ml/libc-alpha/2016-12/msg00914.html
>
> Differential Revision: https://reviews.llvm.org/D28272
>
> Added:
> lld/trunk/test/ELF/Inputs/relocation-copy-relro.s
> lld/trunk/test/ELF/relocation-copy-relro.s
> Modified:
> lld/trunk/ELF/OutputSections.h
> lld/trunk/ELF/Relocations.cpp
> lld/trunk/ELF/Symbols.cpp
> lld/trunk/ELF/Symbols.h
> lld/trunk/ELF/SyntheticSections.cpp
> lld/trunk/ELF/Writer.cpp
> lld/trunk/test/ELF/Inputs/copy-rel-pie.s
>
> Modified: lld/trunk/ELF/OutputSections.h
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.h?rev=291524&r1=291523&r2=291524&view=diff
> ==============================================================================
> --- lld/trunk/ELF/OutputSections.h (original)
> +++ lld/trunk/ELF/OutputSections.h Mon Jan 9 19:21:50 2017
> @@ -206,6 +206,7 @@ template <class ELFT> struct Out {
> static uint8_t First;
> static EhOutputSection<ELFT> *EhFrame;
> static OutputSection<ELFT> *Bss;
> + static OutputSection<ELFT> *BssRelRo;
> static OutputSectionBase *Opd;
> static uint8_t *OpdBuf;
> static PhdrEntry *TlsPhdr;
> @@ -252,6 +253,7 @@ template <class ELFT> uint64_t getHeader
> template <class ELFT> uint8_t Out<ELFT>::First;
> template <class ELFT> EhOutputSection<ELFT> *Out<ELFT>::EhFrame;
> template <class ELFT> OutputSection<ELFT> *Out<ELFT>::Bss;
> +template <class ELFT> OutputSection<ELFT> *Out<ELFT>::BssRelRo;
> template <class ELFT> OutputSectionBase *Out<ELFT>::Opd;
> template <class ELFT> uint8_t *Out<ELFT>::OpdBuf;
> template <class ELFT> PhdrEntry *Out<ELFT>::TlsPhdr;
>
> Modified: lld/trunk/ELF/Relocations.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Relocations.cpp?rev=291524&r1=291523&r2=291524&view=diff
> ==============================================================================
> --- lld/trunk/ELF/Relocations.cpp (original)
> +++ lld/trunk/ELF/Relocations.cpp Mon Jan 9 19:21:50 2017
> @@ -399,7 +399,21 @@ template <class ELFT> static uint32_t ge
> return 1 << TrailingZeros;
> }
>
> -// Reserve space in .bss for copy relocation.
> +template <class ELFT> static bool isReadOnly(SharedSymbol<ELFT> *SS) {
> + typedef typename ELFT::uint uintX_t;
> + typedef typename ELFT::Phdr Elf_Phdr;
> +
> + // Determine if the symbol is read-only by scanning the DSO's program headers.
> + uintX_t Value = SS->Sym.st_value;
> + for (const Elf_Phdr &Phdr : check(SS->file()->getObj().program_headers()))
> + if ((Phdr.p_type == ELF::PT_LOAD || Phdr.p_type == ELF::PT_GNU_RELRO) &&
> + !(Phdr.p_flags & ELF::PF_W) && Value >= Phdr.p_vaddr &&
> + Value < Phdr.p_vaddr + Phdr.p_memsz)
> + return true;
> + return false;
> +}
> +
> +// Reserve space in .bss or .bss.rel.ro for copy relocation.
> template <class ELFT> static void addCopyRelSymbol(SharedSymbol<ELFT> *SS) {
> typedef typename ELFT::uint uintX_t;
> typedef typename ELFT::Sym Elf_Sym;
> @@ -409,10 +423,16 @@ template <class ELFT> static void addCop
> if (SymSize == 0)
> fatal("cannot create a copy relocation for symbol " + toString(*SS));
>
> + // See if this symbol is in a read-only segment. If so, preserve the symbol's
> + // memory protection by reserving space in the .bss.rel.ro section.
> + bool IsReadOnly = isReadOnly(SS);
> + OutputSection<ELFT> *CopySec =
> + IsReadOnly ? Out<ELFT>::BssRelRo : Out<ELFT>::Bss;
> +
> uintX_t Alignment = getAlignment(SS);
> - uintX_t Off = alignTo(Out<ELFT>::Bss->Size, Alignment);
> - Out<ELFT>::Bss->Size = Off + SymSize;
> - Out<ELFT>::Bss->updateAlignment(Alignment);
> + uintX_t Off = alignTo(CopySec->Size, Alignment);
> + CopySec->Size = Off + SymSize;
> + CopySec->updateAlignment(Alignment);
> uintX_t Shndx = SS->Sym.st_shndx;
> uintX_t Value = SS->Sym.st_value;
> // Look through the DSO's dynamic symbol table for aliases and create a
> @@ -425,12 +445,12 @@ template <class ELFT> static void addCop
> Symtab<ELFT>::X->find(check(S.getName(SS->file()->getStringTable()))));
> if (!Alias)
> continue;
> - Alias->OffsetInBss = Off;
> + Alias->CopyIsInBssRelRo = IsReadOnly;
> + Alias->CopyOffset = Off;
> Alias->NeedsCopyOrPltAddr = true;
> Alias->symbol()->IsUsedInRegularObj = true;
> }
> - In<ELFT>::RelaDyn->addReloc(
> - {Target->CopyRel, Out<ELFT>::Bss, SS->OffsetInBss, false, SS, 0});
> + In<ELFT>::RelaDyn->addReloc({Target->CopyRel, CopySec, Off, false, SS, 0});
> }
>
> template <class ELFT>
>
> Modified: lld/trunk/ELF/Symbols.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Symbols.cpp?rev=291524&r1=291523&r2=291524&view=diff
> ==============================================================================
> --- lld/trunk/ELF/Symbols.cpp (original)
> +++ lld/trunk/ELF/Symbols.cpp Mon Jan 9 19:21:50 2017
> @@ -81,7 +81,7 @@ static typename ELFT::uint getSymVA(cons
> return 0;
> if (SS.isFunc())
> return Body.getPltVA<ELFT>();
> - return Out<ELFT>::Bss->Addr + SS.OffsetInBss;
> + return SS.getBssSectionForCopy()->Addr + SS.CopyOffset;
> }
> case SymbolBody::UndefinedKind:
> return 0;
> @@ -97,7 +97,8 @@ SymbolBody::SymbolBody(Kind K, StringRef
> uint8_t Type)
> : SymbolKind(K), NeedsCopyOrPltAddr(false), IsLocal(IsLocal),
> IsInGlobalMipsGot(false), Is32BitMipsGot(false), IsInIplt(false),
> - IsInIgot(false), Type(Type), StOther(StOther), Name(Name) {}
> + IsInIgot(false), CopyIsInBssRelRo(false), Type(Type), StOther(StOther),
> + Name(Name) {}
>
> // Returns true if a symbol can be replaced at load-time by a symbol
> // with the same name defined in other ELF executable or DSO.
> @@ -245,6 +246,12 @@ Undefined<ELFT>::Undefined(StringRefZ Na
> this->File = File;
> }
>
> +template <typename ELFT>
> +OutputSection<ELFT> *SharedSymbol<ELFT>::getBssSectionForCopy() const {
> + assert(needsCopy());
> + return CopyIsInBssRelRo ? Out<ELFT>::BssRelRo : Out<ELFT>::Bss;
> +}
> +
> DefinedCommon::DefinedCommon(StringRef Name, uint64_t Size, uint64_t Alignment,
> uint8_t StOther, uint8_t Type, InputFile *File)
> : Defined(SymbolBody::DefinedCommonKind, Name, /*IsLocal=*/false, StOther,
> @@ -366,6 +373,11 @@ template class elf::Undefined<ELF32BE>;
> template class elf::Undefined<ELF64LE>;
> template class elf::Undefined<ELF64BE>;
>
> +template class elf::SharedSymbol<ELF32LE>;
> +template class elf::SharedSymbol<ELF32BE>;
> +template class elf::SharedSymbol<ELF64LE>;
> +template class elf::SharedSymbol<ELF64BE>;
> +
> template class elf::DefinedRegular<ELF32LE>;
> template class elf::DefinedRegular<ELF32BE>;
> template class elf::DefinedRegular<ELF64LE>;
>
> Modified: lld/trunk/ELF/Symbols.h
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Symbols.h?rev=291524&r1=291523&r2=291524&view=diff
> ==============================================================================
> --- lld/trunk/ELF/Symbols.h (original)
> +++ lld/trunk/ELF/Symbols.h Mon Jan 9 19:21:50 2017
> @@ -123,6 +123,11 @@ public:
> // True if this symbol is in the Igot sub-section of the .got.plt or .got.
> unsigned IsInIgot : 1;
>
> + // True if this is a shared symbol in a read-only segment which requires a
> + // copy relocation. This causes space for the symbol to be allocated in the
> + // .bss.rel.ro section.
> + unsigned CopyIsInBssRelRo : 1;
> +
> // The following fields have the same meaning as the ELF symbol attributes.
> uint8_t Type; // symbol type
> uint8_t StOther; // st_other field value
> @@ -282,13 +287,15 @@ public:
> // This field is a pointer to the symbol's version definition.
> const Elf_Verdef *Verdef;
>
> - // OffsetInBss is significant only when needsCopy() is true.
> - uintX_t OffsetInBss = 0;
> + // CopyOffset is significant only when needsCopy() is true.
> + uintX_t CopyOffset = 0;
>
> // If non-null the symbol has a Thunk that may be used as an alternative
> // destination for callers of this Symbol.
> Thunk<ELFT> *ThunkData = nullptr;
> bool needsCopy() const { return this->NeedsCopyOrPltAddr && !this->isFunc(); }
> +
> + OutputSection<ELFT> *getBssSectionForCopy() const;
> };
>
> // This class represents a symbol defined in an archive file. It is
>
> Modified: lld/trunk/ELF/SyntheticSections.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.cpp?rev=291524&r1=291523&r2=291524&view=diff
> ==============================================================================
> --- lld/trunk/ELF/SyntheticSections.cpp (original)
> +++ lld/trunk/ELF/SyntheticSections.cpp Mon Jan 9 19:21:50 2017
> @@ -1201,10 +1201,12 @@ SymbolTableSection<ELFT>::getOutputSecti
> }
> case SymbolBody::DefinedCommonKind:
> return In<ELFT>::Common->OutSec;
> - case SymbolBody::SharedKind:
> - if (cast<SharedSymbol<ELFT>>(Sym)->needsCopy())
> - return Out<ELFT>::Bss;
> + case SymbolBody::SharedKind: {
> + auto &SS = cast<SharedSymbol<ELFT>>(*Sym);
> + if (SS.needsCopy())
> + return SS.getBssSectionForCopy();
> break;
> + }
> case SymbolBody::UndefinedKind:
> case SymbolBody::LazyArchiveKind:
> case SymbolBody::LazyObjectKind:
>
> Modified: lld/trunk/ELF/Writer.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=291524&r1=291523&r2=291524&view=diff
> ==============================================================================
> --- lld/trunk/ELF/Writer.cpp (original)
> +++ lld/trunk/ELF/Writer.cpp Mon Jan 9 19:21:50 2017
> @@ -250,6 +250,8 @@ template <class ELFT> void Writer<ELFT>:
> // Create singleton output sections.
> Out<ELFT>::Bss =
> make<OutputSection<ELFT>>(".bss", SHT_NOBITS, SHF_ALLOC | SHF_WRITE);
> + Out<ELFT>::BssRelRo = make<OutputSection<ELFT>>(".bss.rel.ro", SHT_NOBITS,
> + SHF_ALLOC | SHF_WRITE);
> In<ELFT>::DynStrTab = make<StringTableSection<ELFT>>(".dynstr", true);
> In<ELFT>::Dynamic = make<DynamicSection<ELFT>>();
> Out<ELFT>::EhFrame = make<EhOutputSection<ELFT>>();
> @@ -498,6 +500,8 @@ template <class ELFT> bool elf::isRelroS
> return true;
> if (In<ELFT>::MipsGot && Sec == In<ELFT>::MipsGot->OutSec)
> return true;
> + if (Sec == Out<ELFT>::BssRelRo)
> + return true;
> StringRef S = Sec->getName();
> return S == ".data.rel.ro" || S == ".ctors" || S == ".dtors" || S == ".jcr" ||
> S == ".eh_frame" || S == ".openbsd.randomdata";
> @@ -1079,6 +1083,8 @@ template <class ELFT> void Writer<ELFT>:
> template <class ELFT> void Writer<ELFT>::addPredefinedSections() {
> if (Out<ELFT>::Bss->Size > 0)
> OutputSections.push_back(Out<ELFT>::Bss);
> + if (Out<ELFT>::BssRelRo->Size > 0)
> + OutputSections.push_back(Out<ELFT>::BssRelRo);
>
> auto OS = dyn_cast_or_null<OutputSection<ELFT>>(findSection(".ARM.exidx"));
> if (OS && !OS->Sections.empty() && !Config->Relocatable)
>
> Modified: lld/trunk/test/ELF/Inputs/copy-rel-pie.s
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/Inputs/copy-rel-pie.s?rev=291524&r1=291523&r2=291524&view=diff
> ==============================================================================
> --- lld/trunk/test/ELF/Inputs/copy-rel-pie.s (original)
> +++ lld/trunk/test/ELF/Inputs/copy-rel-pie.s Mon Jan 9 19:21:50 2017
> @@ -1,9 +1,11 @@
> +.data
> .global foo
> .type foo, @object
> .size foo, 4
> foo:
> .long 0
>
> +.text
> .global bar
> .type bar, @function
> bar:
>
> Added: lld/trunk/test/ELF/Inputs/relocation-copy-relro.s
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/Inputs/relocation-copy-relro.s?rev=291524&view=auto
> ==============================================================================
> --- lld/trunk/test/ELF/Inputs/relocation-copy-relro.s (added)
> +++ lld/trunk/test/ELF/Inputs/relocation-copy-relro.s Mon Jan 9 19:21:50 2017
> @@ -0,0 +1,13 @@
> +.rodata
> +.globl a
> +.size a, 4
> +.type a, @object
> +a:
> +.word 1
> +
> +.section .data.rel.ro,"aw",%progbits
> +.globl b
> +.size b, 4
> +.type b, @object
> +b:
> +.word 2
>
> Added: lld/trunk/test/ELF/relocation-copy-relro.s
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/relocation-copy-relro.s?rev=291524&view=auto
> ==============================================================================
> --- lld/trunk/test/ELF/relocation-copy-relro.s (added)
> +++ lld/trunk/test/ELF/relocation-copy-relro.s Mon Jan 9 19:21:50 2017
> @@ -0,0 +1,32 @@
> +// REQUIRES: x86
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/relocation-copy-relro.s -o %t2.o
> +// RUN: ld.lld -shared %t2.o -o %t.so
> +// RUN: ld.lld %t.o %t.so -o %t3
> +// RUN: llvm-readobj -program-headers -s -r %t3 | FileCheck %s
> +
> +// CHECK: Name: .bss.rel.ro (48)
> +// CHECK-NEXT: Type: SHT_NOBITS (0x8)
> +// CHECK-NEXT: Flags [ (0x3)
> +// CHECK-NEXT: SHF_ALLOC (0x2)
> +// CHECK-NEXT: SHF_WRITE (0x1)
> +// CHECK-NEXT: ]
> +// CHECK-NEXT: Address: 0x2020B0
> +// CHECK-NEXT: Offset: 0x20B0
> +// CHECK-NEXT: Size: 8
> +
> +// CHECK: 0x2020B0 R_X86_64_COPY a 0x0
> +// CHECK: 0x2020B4 R_X86_64_COPY b 0x0
> +
> +// CHECK: Type: PT_GNU_RELRO (0x6474E552)
> +// CHECK-NEXT: Offset: 0x2000
> +// CHECK-NEXT: VirtualAddress: 0x202000
> +// CHECK-NEXT: PhysicalAddress: 0x202000
> +// CHECK-NEXT: FileSize: 176
> +// CHECK-NEXT: MemSize: 4096
> +
> +.text
> +.global _start
> +_start:
> +movl $1, a
> +movl $2, b
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list