[PATCH] D40029: [ELF] Only add contiguous isRelro sections to PT_GNU_RELRO PHDR
Peter Smith via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 14 08:16:23 PST 2017
peter.smith created this revision.
Herald added a subscriber: arichardson.
Some linker scripts, in particular the default ld.bfd linker script can match the InX::BssRelRo section in the .bss OutputSection . This causes the OutputSections for which isRelroSection() is true to be non-contiguous in the memory map. If we add all the OutputSections to the PT_GNU_RELRO PHDR then the size of said PHDR will include read-write sections. This may cause segfaults if the PT_GNU_RELRO PHDR covers read-write data that is written to.
This change only adds OutputSections for which isRelroSection() is true to the PT_GNU_RELRO PHDR if they are contiguous in the memory map. Later OutputSections for which isRelroSection() is true will be read-write.
Fixes PR35265
Implementation Notes:
I found this problem when using the default linker script from ld.bfd (script obtainable via ld --verbose). Even with an empty main the program would segfault when writing back the result of lazy PLT resolution into the .got.plt. The segfault was caused by the .got.plt section being marked read-only as it was covered by the PT_GNU_RELRO PHDR. The reason it was covered was due to the zero sized InX::BssRelRo section matching the .bss Output Section in the linker script extract below. As the current code calculates the size of the PT_GNU_RELRO PHDR as P->p_memsz = Last->Addr + Last->Size - First->Addr; we get the size of .got.plt and .data included in the PT_GNU_RELRO PHDR as InX::BssRelRo is Last.
The impact of the fix is that if the linker script does produce non-contiguous isRelroSections then only the first contiguous chunk will be covered by the PT_GNU_RELRO PHDR. Any remaining isRelroSections will be read-write in the output. I've chosen this in favour of compatibility of the default ld.bfd linker script; however a stricter view would be that it is an error if isRelroSections are not contiguous as this could be a potential security hole. I note that it was a 0 sized InX::BssRelRo matching in *(.bss .bss.* .gnu.linkonce.b.*) that passed the if (needsPtLoad(Sec) && isRelroSection(Sec)) test. Perhaps if non-contiguous isRelroSections is an error then 0 sized InX::BssRelRo can be an exception as this is not an obvious user-error.
.got : { *(.got) *(.igot) }
. = DATA_SEGMENT_RELRO_END (SIZEOF (.got.plt) >= 24 ? 24 : 0, .);
.got.plt : { *(.got.plt) *(.igot.plt) }
.data :
{
*(.data .data.* .gnu.linkonce.d.*)
SORT(CONSTRUCTORS)
}
.data1 : { *(.data1) }
_edata = .; PROVIDE (edata = .);
. = .;
__bss_start = .;
.bss :
{
*(.dynbss)
*(.bss .bss.* .gnu.linkonce.b.*)
*(COMMON)
/* Align here to ensure that the .bss section occupies space up to
_end. Align after .bss to ensure correct alignment even if the
.bss section disappears because there are no input sections.
FIXME: Why do we need it? When there is no .bss section, we don't
pad the .data section. */
. = ALIGN(. != 0 ? 64 / 8 : 1);
}
https://reviews.llvm.org/D40029
Files:
ELF/Writer.cpp
test/ELF/linkerscript/non-contiguous-relro.s
Index: test/ELF/linkerscript/non-contiguous-relro.s
===================================================================
--- /dev/null
+++ test/ELF/linkerscript/non-contiguous-relro.s
@@ -0,0 +1,53 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/shared.s -o %t2.o
+# RUN: ld.lld -shared %t2.o -o %t2.so
+
+# The following linker script will match the isRelroSection() .bss.rel.ro in
+# .bss. This will mean that the isRelroSections are non contiguous in the
+# memory map. The PT_GNU_RELRO can only match a contiguous range of bytes so
+# we exclude the .bss.rel.ro section.
+
+# RUN: echo "SECTIONS { \
+# RUN: . = SIZEOF_HEADERS; \
+# RUN: .plt : { *(.plt) } \
+# RUN: .text : { *(.text) } \
+# RUN: . = DATA_SEGMENT_ALIGN (CONSTANT (MAXPAGESIZE), CONSTANT (COMMONPAGESIZE)); \
+# RUN: .dynamic : { *(.dynamic) } \
+# RUN: .got : { *(.got) } \
+# RUN: . = DATA_SEGMENT_RELRO_END (1 ? 24 : 0, .); \
+# RUN: .got.plt : { *(.got.plt) } \
+# RUN: .data : { *(.data) } \
+# RUN: .bss : { *(.bss .bss.*) } \
+# RUN: . = DATA_SEGMENT_END (.); \
+# RUN: }" > %t.script
+
+# RUN: ld.lld --hash-style=sysv -z relro %t1.o %t2.so --script %t.script -o %t2
+# RUN: llvm-readobj -program-headers %t2 | FileCheck %s
+
+# CHECK: Type: PT_GNU_RELRO
+# CHECK-NEXT: Offset: 0x1000
+# CHECK-NEXT: VirtualAddress: 0x1000
+# CHECK-NEXT: PhysicalAddress: 0x1000
+# CHECK-NEXT: FileSize:
+# CHECK-NEXT: MemSize: 4096
+# CHECK-NEXT: Flags [
+# CHECK-NEXT: PF_R
+# CHECK-NEXT: ]
+# CHECK-NEXT: Alignment: 1
+# CHECK-NEXT: }
+
+
+.global _start
+_start:
+ .long bar
+ jmp *bar2 at GOTPCREL(%rip)
+
+.section .data,"aw"
+# At least one page in size, if it were incorrectly included in PT_GNU_RELRO
+# it would affect MemSize of PT_GNU_RELRO.
+.space 4 * 1024
+
+.zero 4
+.section .foo,"aw"
+.section .bss.rel.ro,"", at nobits
Index: ELF/Writer.cpp
===================================================================
--- ELF/Writer.cpp
+++ ELF/Writer.cpp
@@ -1503,9 +1503,18 @@
// PT_GNU_RELRO includes all sections that should be marked as
// read-only by dynamic linker after proccessing relocations.
PhdrEntry *RelRo = make<PhdrEntry>(PT_GNU_RELRO, PF_R);
- for (OutputSection *Sec : OutputSections)
- if (needsPtLoad(Sec) && isRelroSection(Sec))
+ bool SeenRelro = false;
+ for (OutputSection *Sec : OutputSections) {
+ if (!needsPtLoad(Sec))
+ continue;
+ if (isRelroSection(Sec)) {
+ SeenRelro = true;
RelRo->add(Sec);
+ } else if (SeenRelro)
+ // The Relro sections added must be contiguous in memory. Any later
+ // Relro sections will be read-write.
+ break;
+ }
if (RelRo->FirstSec)
Ret.push_back(RelRo);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40029.122844.patch
Type: text/x-patch
Size: 2844 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171114/6f32a27f/attachment.bin>
More information about the llvm-commits
mailing list