[ELF] Support custom sections between DATA_SEGMENT_ALIGN and DATA_SEGMENT_RELRO_END

[MaskRay]

Summary

We currently hard code RELRO sections. When a custom section is between
DATA_SEGMENT_ALIGN and DATA_SEGMENT_RELRO_END, we may report a spurious
error: section: ... is not contiguous with other relro sections. GNU ld
makes such sections RELRO.

glibc recently switched to default --with-default-link=no. This configuration
places __libc_atexit and others between DATA_SEGMENT_ALIGN and
DATA_SEGMENT_RELRO_END. This patch allows such a ld.bfd --verbose
linker script to be fed into lld.

[MaskRay]

Related:
I think the original is not contiguous with other relro sections error from https://reviews.llvm.org/D40359 is useful.

I remember that ~2 years some folks asked on llvm-dev how to make a custom section RELRO but that discussion did not lead to any action item.
When building glibc this feature rises again (https://sourceware.org/pipermail/libc-alpha/2022-April/138284.html)
(specifying a full SECTIONS command is clumsy and lld doesn't support dumping its built-in rules).
The best I can come up with is

SECTIONS {
         __libc_subfreeres : { *(__libc_subfreeres) }
         __libc_atexit : { *(__libc_atexit) }
         __libc_IO_vtables : { *(__libc_IO_vtables) }
} INSERT BEFORE .data.rel.ro;

But it will trigger the is not contiguous with other relro sections error.
I wonder whether lld should (un)support this.

[peter.smith]

Looks sensible to me. One quick question, how does orphan placement relate to RELRO. For example if we make RELRO defined as [first relro section start, last relro section end) between DATA_SECTION_ALIGN and DATA_SECTION_RELRO_END, is there any danger of orphan placement putting a non-relro section inside that range? That case isn't safe for RW as it could be written to at runtime.

I suppose the alternative is to add these custom sections to the isRelroSection() although I don't think that is scalable. Another possible source of custom RELRO sections is #pragma clang section relro="foo" and __attribute__((section("foo"))).

In D124656#3481926, @MaskRay wrote:

Related:
I think the original is not contiguous with other relro sections error from https://reviews.llvm.org/D40359 is useful.

I remember that ~2 years some folks asked on llvm-dev how to make a custom section RELRO but that discussion did not lead to any action item.
When building glibc this feature rises again (https://sourceware.org/pipermail/libc-alpha/2022-April/138284.html)
(specifying a full SECTIONS command is clumsy and lld doesn't support dumping its built-in rules).
The best I can come up with is

SECTIONS {
         __libc_subfreeres : { *(__libc_subfreeres) }
         __libc_atexit : { *(__libc_atexit) }
         __libc_IO_vtables : { *(__libc_IO_vtables) }
} INSERT BEFORE .data.rel.ro;

But it will trigger the is not contiguous with other relro sections error.
I wonder whether lld should (un)support this.

It maybe an area like orphan sections where a policy can be given. Given that many dynamic loaders can only support one RELRO region we've got a number of choices if we see disjoint RELRO. If there are legitimate alternatives to the default then we could support them via a command line option. My thoughts on the policies when there is no linker script with DATA_SEGMENT_RELRO_END:

• Error: when RELRO isn't contiguous, we don't know enough about the non-relro sections to know if it is safe or secure enough to promote non-relro to relro
• Promote-RO: In theory making an RO section RELRO is safe, with only a minor drop in security. Error if the section is RW as we can't know if that is safe to make RELRO
• First: Make the first contiguous sections RELRO and make all following RELRO sections RW. This could result in loss of expected security properties.
• All: RELRO is from [first RELRO section start, last RELRO section end). This could cause runtime errors if a RW section in the RELRO region is written to. This is the trust me, I know what I'm doing option.

I think promote-ro could be a safe default.

lld/test/ELF/linkerscript/data-segment-relro.test

80

Would be good to add a test case for orphan sections to make sure that we don't add a RW section into the middle of the RELRO sections, as this could be unsafe. I'm happy that the user takes responsibility for everything they've put in the linker script explicitly.

[MaskRay]

add orphan section tests

[MaskRay]

Thanks for the detailed reply. With this patch, users needing custom RELRO sections can specify a full SECTIONS command.

The current Error behavior seems fine.
I believe Promote-RO is technically possible (only addresses of PT_GNU_RELRO are used; the pt_offset is ignored) but may look strange and may confuse binary manipulation tools (llvm-objcopy, objcopy).

All: RELRO is from [first RELRO section start, last RELRO section end). This could cause runtime errors if a RW section in the RELRO region is written to. This is the trust me, I know what I'm doing option.

This may probably be the most convenient approach if the user just wants to specify a linker script fragment (e.g.SECTIONS { ... } INSERT [BEFORE|AFTER] ...;).
But I agree that it is not a safe default.

---

Given that there are not demanding needs, perhaps not doing anything is fine for now.

[peter.smith]

LGTM, thanks for the update.

lld/ELF/OutputSections.h

public:
   bool usedInExpression = false;
   bool inOverlay = false;
 
   // Tracks whether the section has ever had an input section added to it, even
   // if the section was later removed (e.g. because it is a synthetic section
   // that wasn't needed). This is needed for orphan placement.
   bool hasInputSections = false;
 
+  // The output section description is specified between DATA_SEGMENT_ALIGN and
+  // DATA_RELRO_END.
+  bool relro = false;
+
   void finalize();
   template <class ELFT> void writeTo(uint8_t *buf);
   // Check that the addends for dynamic relocations were written correctly.
   void checkDynRelAddends(const uint8_t *bufStart);
   template <class ELFT> void maybeCompress();
 
   void sort(llvm::function_ref<int(InputSectionBase *s)> order);
   void sortInitFini();

lld/ELF/ScriptParser.cpp

private:
   void readVersionDeclaration(StringRef verStr);
 
   std::pair<SmallVector<SymbolVersion, 0>, SmallVector<SymbolVersion, 0>>
   readSymbols();
 
   // True if a script being read is in the --sysroot directory.
   bool isUnderSysroot = false;
 
+  bool seenDataAlign = false;
+  bool seenRelroEnd = false;
+
   // A set to detect an INCLUDE() cycle.
   StringSet<> seen;
 };
 } // namespace
 
 static StringRef unquote(StringRef s) {
   if (s.startswith("\""))
     return s.substr(1, s.size() - 2);
if (tok == "OVERLAY") {
       continue;
     }
 
     if (SectionCommand *cmd = readAssignment(tok))
       v.push_back(cmd);
     else
       v.push_back(readOutputSectionDescription(tok));
   }
+
+  // If DATA_SEGMENT_RELRO_END is absent, for sections after DATA_SEGMENT_ALIGN,
+  // the relro fields should be cleared.
+  if (!seenRelroEnd)
+    for (SectionCommand *cmd : v)
+      if (auto *osd = dyn_cast<OutputDesc>(cmd))
+        osd->osec.relro = false;
+
   script->sectionCommands.insert(script->sectionCommands.end(), v.begin(),
                                  v.end());
 
   if (atEOF() || !consume("INSERT")) {
     script->hasSectionsCommand = true;
     return;
   }
 
osd->osec.commands.push_back(
         readInputSectionRules(next(), withFlags, withoutFlags));
   }
   return osd;
 }
 
 OutputDesc *ScriptParser::readOutputSectionDescription(StringRef outSec) {
   OutputDesc *cmd = script->createOutputSection(outSec, getCurrentLocation());
   OutputSection *osec = &cmd->osec;
+  // Maybe relro. Will reset to false if DATA_SEGMENT_RELRO_END is absent.
+  osec->relro = seenDataAlign && !seenRelroEnd;
 
   size_t symbolsReferenced = script->referencedSymbols.size();
 
   if (peek() != ":")
     readSectionAddressType(osec);
   expect(":");
 
   std::string location = getCurrentLocation();
Expr ScriptParser::readPrimary() {
   if (tok == "CONSTANT")
     return readConstant();
   if (tok == "DATA_SEGMENT_ALIGN") {
     expect("(");
     Expr e = readExpr();
     expect(",");
     readExpr();
     expect(")");
+    seenDataAlign = true;
     return [=] {
       return alignTo(script->getDot(), std::max((uint64_t)1, e().getValue()));
     };
   }
   if (tok == "DATA_SEGMENT_END") {
     expect("(");
     expect(".");
     expect(")");
     return [] { return script->getDot(); };
   }
   if (tok == "DATA_SEGMENT_RELRO_END") {
     // GNU linkers implements more complicated logic to handle
     // DATA_SEGMENT_RELRO_END. We instead ignore the arguments and
     // just align to the next page boundary for simplicity.
     expect("(");
     readExpr();
     expect(",");
     readExpr();
     expect(")");
+    seenRelroEnd = true;
     Expr e = getPageSize();
     return [=] { return alignTo(script->getDot(), e().getValue()); };
   }
   if (tok == "DEFINED") {
     StringRef name = unquote(readParenLiteral());
     return [=] {
       Symbol *b = symtab->find(name);
       return (b && b->isDefined()) ? 1 : 0;

lld/ELF/Writer.cpp

 // processing dynamic relocations to enhance security. PT_GNU_RELRO
 // is defined for that.
 //
 // This function returns true if a section needs to be put into a
 // PT_GNU_RELRO segment.
 static bool isRelroSection(const OutputSection *sec) {
   if (!config->zRelro)
     return false;
+  if (sec->relro)
+    return true;
 
   uint64_t flags = sec->flags;
 
   // Non-allocatable or non-writable sections don't need RELRO because
   // they are not writable or not even mapped to memory in the first place.
   // RELRO is for sections that are essentially read-only but need to
   // be writable only at process startup to allow dynamic linker to
   // apply relocations.

lld/test/ELF/linkerscript/data-segment-relro.test

 # RUN: ld.lld --hash-style=sysv -z norelro %t/a.o %t/b.so -T %t/1.t -o %t/a1
 # RUN: llvm-readelf -S -l %t/a1 | FileCheck %s --check-prefixes=CHECK,CHECK1
 
 # RUN: ld.lld --hash-style=sysv -z relro %t/a.o %t/b.so -T %t/1.t -o %t/a2
 # RUN: llvm-readelf -S -l %t/a2 | FileCheck %s --check-prefixes=CHECK,CHECK2
 
 # CHECK:      Name           Type     Address          Off      Size ES Flg
 # CHECK-NEXT:                NULL     {{.*}}
-# CHECK:      .dynamic       DYNAMIC  0000000000001000 001000 0000f0 10  WA
-# CHECK-NEXT: .got           PROGBITS 00000000000010f0 0010f0 000008 00  WA
-# CHECK-NEXT: __libc_atexit  PROGBITS 0000000000002000 002000 000008 00  WA
-# CHECK-NEXT: .got.plt       PROGBITS 0000000000002008 002008 000020 00  WA
+# CHECK:      .orphan.ro     PROGBITS {{.*}}                              A
+# CHECK:      .dynamic       DYNAMIC  {{.*}}                             WA
+# CHECK-NEXT: __libc_atexit  PROGBITS {{.*}}                             WA
+# CHECK-NEXT: .got           PROGBITS {{.*}}                             WA
+# CHECK-NEXT: .got.plt       PROGBITS {{.*}}                             WA
+# CHECK:      .orphan.rw     PROGBITS {{.*}}                             WA
 
 # CHECK1:      Program Headers:
 # CHECK1-NOT:  GNU_RELRO
 
 # CHECK2:      Program Headers:
 # CHECK2-NEXT:   Type
 # CHECK2-NEXT:   PHDR
 # CHECK2-NEXT:   LOAD
 # CHECK2-NEXT:   LOAD
 # CHECK2-NEXT:   LOAD
 # CHECK2-NEXT:   LOAD
 # CHECK2-NEXT:   DYNAMIC
 # CHECK2-NEXT:   GNU_RELRO
 # CHECK2-NEXT:   GNU_STACK
 
 # CHECK2:      Section to Segment mapping:
-# CHECK2:        06     .dynamic .got {{$}}
+# CHECK2:        06     .dynamic __libc_atexit .got {{$}}
 
 # RUN: sed '/DATA_SEGMENT_RELRO_END/d' %t/1.t > %t/2.t
-# RUN: ld.lld %t/a.o %t/b.so -T %t/2.t -o /dev/null
+# RUN: not ld.lld %t/a.o %t/b.so -T %t/2.t -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR
+
+# ERR: error: section: .got is not contiguous with other relro sections
 
 #--- a.s
 .global _start
 _start:
   .long bar
   jmp *bar2@GOTPCREL(%rip)
 
 .section .data,"aw"
 .quad 0
 
 .zero 4
 .section .foo,"aw"
 .section .bss,"",@nobits
 
 .section __libc_atexit,"aw",@progbits
 .dc.a __libc_atexit
 
+.section .orphan.ro,"a",@progbits
+.dc.a 0
+
+.section .orphan.rw,"aw",@progbits
+.dc.a .orphan.rw
+
 #--- 1.t
 SECTIONS {
   . = SIZEOF_HEADERS;
 
   .plt  : { *(.plt) }
   .text : { *(.text) }
 
   . = DATA_SEGMENT_ALIGN (CONSTANT (MAXPAGESIZE), CONSTANT (COMMONPAGESIZE));
 
   .dynamic : { *(.dynamic) }
+  ## The custom section __libc_atexit is made relro.
+  __libc_atexit : { *(__libc_atexit) }
   .got : { *(.got) }
 
   . = DATA_SEGMENT_RELRO_END (1 ? 24 : 0, .);
 
-  __libc_atexit : { *(__libc_atexit) }
   .got.plt : { *(.got.plt) }
   .data : { *(.data) }
   .bss : { *(.bss) }
 
   . = DATA_SEGMENT_END (.);
 
   .comment 0 : { *(.comment) }
 }