[ELF] Align the end of PT_GNU_RELRO associated PT_LOAD to a common-page-size boundary #66042
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lld-elf ChangesClose #57618: currently we align the end of PT_GNU_RELRO to a common-page-size Close #65002: GNU ld from 2.39 onwards aligns the end of PT_GNU_RELRO to a To prevent strip from corrupting PT_LOAD program headers, add a padding section Our RELRO section layouts are different from GNU ld, making 18 tests need updating as there are lots of change detectors.
-- Patch is 24.34 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66042.diff 25 Files Affected:
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 9219314111610d1..9d167293574fa64 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1586,8 +1586,8 @@ static void readConfigs(opt::InputArgList &args) {
// Page alignment can be disabled by the -n (--nmagic) and -N (--omagic).
// As PT_GNU_RELRO relies on Paging, do not create it when we have disabled
- // it.
- if (config->nmagic || config->omagic)
+ // it. Also disable RELRO for -r.
+ if (config->nmagic || config->omagic || config->relocatable)
config->zRelro = false;
std::tie(config->buildId, config->buildIdVector) = getBuildId(args);
diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index 2b9f28ce68d685d..2973ff43106edc3 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -1079,6 +1079,11 @@ void LinkerScript::assignOffsets(OutputSection *sec) {
}
}
+ // If .relro_padding is present, round up the end to a max-page-size boundary
+ // to protect the last page.
+ if (in.relroPadding && sec == in.relroPadding->getParent())
+ expandOutputSection(alignToPowerOf2(dot, config->maxPageSize) - dot);
+
// Non-SHF_ALLOC sections do not affect the addresses of other OutputSections
// as they are not part of the process image.
if (!(sec->flags & SHF_ALLOC)) {
@@ -1160,6 +1165,7 @@ void LinkerScript::adjustOutputSections() {
uint64_t flags = SHF_ALLOC;
SmallVector defPhdrs;
+ bool seenRelro = false;
for (SectionCommand *&cmd : sectionCommands) {
if (!isa(cmd))
continue;
@@ -1196,9 +1202,17 @@ void LinkerScript::adjustOutputSections() {
if (sec->sectionIndex != UINT32_MAX)
maybePropagatePhdrs(*sec, defPhdrs);
+ // Discard .relro_padding if we have not seen one non-NOBITS RELRO section.
+ // Note: when .tbss is the only RELRO section, it's difficult to pick a
+ // suitable padding size (see computeFilOffset). For simplicity, don't
+ // retain .relro_padding in this case.
+ if (in.relroPadding && in.relroPadding->getParent() == sec && !seenRelro)
+ discardable = true;
if (discardable) {
sec->markDead();
cmd = nullptr;
+ } else {
+ seenRelro |= sec->relro && sec->type != SHT_NOBITS;
}
}
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 09090835af4a621..f412efa36480284 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -2688,6 +2688,10 @@ size_t IBTPltSection::getSize() const {
bool IBTPltSection::isNeeded() const { return in.plt->getNumEntries() > 0; }
+RelroPaddingSection::RelroPaddingSection()
+ : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_NOBITS, 1, ".relro_padding") {
+}
+
// The string hash function for .gdb_index.
static uint32_t computeGdbHash(StringRef s) {
uint32_t h = 0;
@@ -3839,6 +3843,7 @@ void InStruct::reset() {
got.reset();
gotPlt.reset();
igotPlt.reset();
+ relroPadding.reset();
armCmseSGSection.reset();
ppc64LongBranchTarget.reset();
mipsAbiFlags.reset();
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index 21a3f768ffcf8e1..1e527759946bef7 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -778,6 +778,14 @@ class IBTPltSection : public SyntheticSection {
size_t getSize() const override;
};
+// Used to align the end of PT_GNU_RELRO to a max-page-size boundary.
+class RelroPaddingSection final : public SyntheticSection {
+public:
+ RelroPaddingSection();
+ size_t getSize() const override { return 0; }
+ void writeTo(uint8_t *buf) override {}
+};
+
class GdbIndexSection final : public SyntheticSection {
public:
struct AddressEntry {
@@ -1333,6 +1341,7 @@ struct InStruct {
std::unique_ptr got;
std::unique_ptr gotPlt;
std::unique_ptr igotPlt;
+ std::unique_ptr relroPadding;
std::unique_ptr armCmseSGSection;
std::unique_ptr ppc64LongBranchTarget;
std::unique_ptr mipsAbiFlags;
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 255f8c334b969bc..f4f81539fe9f2cc 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -458,6 +458,12 @@ template void elf::createSyntheticSections() {
add(*in.gotPlt);
in.igotPlt = std::make_unique();
add(*in.igotPlt);
+ // In the absence of PHDRS/SECTIONS commands. add .relro_padding.
+ if (config->zRelro && script->phdrsCommands.empty() &&
+ !script->hasSectionsCommand) {
+ in.relroPadding = std::make_unique();
+ add(*in.relroPadding);
+ }
if (config->emachine == EM_ARM) {
in.armCmseSGSection = std::make_unique();
@@ -818,6 +824,9 @@ static bool isRelroSection(const OutputSection *sec) {
if (sec == in.gotPlt->getParent())
return config->zNow;
+ if (in.relroPadding && sec == in.relroPadding->getParent())
+ return true;
+
// .dynamic section contains data for the dynamic linker, and
// there's no need to write to it at runtime, so it's better to put
// it into RELRO.
@@ -857,7 +866,7 @@ enum RankFlags {
RF_BSS = 1 << 7,
};
-static unsigned getSectionRank(const OutputSection &osec) {
+static unsigned getSectionRank(OutputSection &osec) {
unsigned rank = osec.partition * RF_PARTITION;
// We want to put section specified by -T option first, so we
@@ -920,7 +929,9 @@ static unsigned getSectionRank(const OutputSection &osec) {
// TLS sections directly before the other RELRO sections.
if (!(osec.flags & SHF_TLS))
rank |= RF_NOT_TLS;
- if (!isRelroSection(&osec))
+ if (isRelroSection(&osec))
+ osec.relro = true;
+ else
rank |= RF_NOT_RELRO;
// Place .ldata and .lbss after .bss. Making .bss closer to .text alleviates
// relocation overflow pressure.
@@ -2336,6 +2347,7 @@ SmallVector Writer::createPhdrs(Partition &part) {
relroEnd = sec;
}
}
+ relRo->p_align = 1;
for (OutputSection *sec : outputSections) {
if (!needsPtLoad(sec))
@@ -2679,16 +2691,6 @@ template void Writer::setPhdrs(Partition &part) {
if (!p->hasLMA)
p->p_paddr = first->getLMA();
}
-
- if (p->p_type == PT_GNU_RELRO) {
- p->p_align = 1;
- // musl/glibc ld.so rounds the size down, so we need to round up
- // to protect the last page. This is a no-op on FreeBSD which always
- // rounds up.
- p->p_memsz =
- alignToPowerOf2(p->p_offset + p->p_memsz, config->commonPageSize) -
- p->p_offset;
- }
}
}
diff --git a/lld/docs/ReleaseNotes.rst b/lld/docs/ReleaseNotes.rst
index 1590879416853e0..dc9d3fe7abbe645 100644
--- a/lld/docs/ReleaseNotes.rst
+++ b/lld/docs/ReleaseNotes.rst
@@ -29,6 +29,8 @@ ELF Improvements
* ``--fat-lto-objects`` option is added to support LLVM FatLTO.
Without ``--fat-lto-objects``, LLD will link LLVM FatLTO objects using the
relocatable object file. (`D146778 `_)
+* the end of the ``PT_GNU_RELRO`` segment is now aligned to a max-page-size
+ boundary.
Breaking changes
diff --git a/lld/test/ELF/aarch64-relro.s b/lld/test/ELF/aarch64-relro.s
index 4047382c8aa1bd4..4223156df996eef 100644
--- a/lld/test/ELF/aarch64-relro.s
+++ b/lld/test/ELF/aarch64-relro.s
@@ -8,7 +8,7 @@
# CHECK-NEXT: VirtualAddress: 0x220190
# CHECK-NEXT: PhysicalAddress:
# CHECK-NEXT: FileSize:
-# CHECK-NEXT: MemSize: 3696
+# CHECK-NEXT: MemSize: 65136
.section .data.rel.ro,"aw",%progbits
.byte 1
diff --git a/lld/test/ELF/arm-execute-only.s b/lld/test/ELF/arm-execute-only.s
index 483d33d19fcd4a6..5f7f823d3ed5a6b 100644
--- a/lld/test/ELF/arm-execute-only.s
+++ b/lld/test/ELF/arm-execute-only.s
@@ -13,7 +13,7 @@
// CHECK: LOAD 0x000000 0x00000000 0x00000000 0x0016d 0x0016d R 0x10000
// CHECK: LOAD 0x000170 0x00010170 0x00010170 0x{{.*}} 0x{{.*}} R E 0x10000
// CHECK: LOAD 0x000174 0x00020174 0x00020174 0x{{.*}} 0x{{.*}} E 0x10000
-// CHECK: LOAD 0x000178 0x00030178 0x00030178 0x00038 0x00038 RW 0x10000
+// CHECK: LOAD 0x000178 0x00030178 0x00030178 0x00038 0x0fe88 RW 0x10000
// CHECK: 01 .dynsym .gnu.hash .hash .dynstr
// CHECK: 02 .text
@@ -22,7 +22,7 @@
// DIFF: LOAD 0x000000 0x00000000 0x00000000 0x0014d 0x0014d R 0x10000
// DIFF: LOAD 0x000150 0x00010150 0x00010150 0x0000c 0x0000c R E 0x10000
-// DIFF: LOAD 0x00015c 0x0002015c 0x0002015c 0x00038 0x00038 RW 0x10000
+// DIFF: LOAD 0x00015c 0x0002015c 0x0002015c 0x00038 0x0fea4 RW 0x10000
// DIFF: 01 .dynsym .gnu.hash .hash .dynstr
// DIFF: 02 .text .foo
diff --git a/lld/test/ELF/end-dso-defined.s b/lld/test/ELF/end-dso-defined.s
index 0b5e977296d1042..d0c143c25148e25 100644
--- a/lld/test/ELF/end-dso-defined.s
+++ b/lld/test/ELF/end-dso-defined.s
@@ -21,16 +21,16 @@
# CHECK-NEXT: AddressAlignment:
# CHECK-NEXT: EntrySize:
# CHECK-NEXT: SectionData (
-# CHECK-NEXT: 0000: 08232000 00000000 08232000 00000000
+# CHECK-NEXT: 0000: 00302000 00000000 00302000 00000000
# CHECK-NEXT: )
# CHECK: Symbol {
# CHECK: Name: _end
-# CHECK-NEXT: Value: 0x202308
+# CHECK-NEXT: Value: 0x203000
# CHECK: Symbol {
# CHECK: Name: end
-# CHECK-NEXT: Value: 0x202308
+# CHECK-NEXT: Value: 0x203000
.global _start
_start:
diff --git a/lld/test/ELF/linkerscript/data-segment-relro.test b/lld/test/ELF/linkerscript/data-segment-relro.test
index 8aba8acb466f6eb..df819e25cc8caaf 100644
--- a/lld/test/ELF/linkerscript/data-segment-relro.test
+++ b/lld/test/ELF/linkerscript/data-segment-relro.test
@@ -26,15 +26,16 @@
# 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-NEXT: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
+# CHECK2-NEXT: PHDR 0x000040
+# CHECK2-NEXT: LOAD 0x000000
+# CHECK2-NEXT: LOAD 0x0002b0
+# CHECK2-NEXT: LOAD 0x001000 0x0000000000001000 0x0000000000001000 0x000100 0x000100 RW 0x1000
+# CHECK2-NEXT: LOAD 0x002000 0x0000000000002000 0x0000000000002000 0x000034 0x000034 RW 0x1000
+# CHECK2-NEXT: DYNAMIC 0x001000 0x0000000000001000 0x0000000000001000 0x0000f0 0x0000f0 RW 0x8
+# CHECK2-NEXT: GNU_RELRO 0x001000 0x0000000000001000 0x0000000000001000 0x000100 0x000100 R 0x1
+# CHECK2-NEXT: GNU_STACK 0x000000
+
# CHECK2: Section to Segment mapping:
# CHECK2: 06 .dynamic __libc_atexit .got {{$}}
diff --git a/lld/test/ELF/linkerscript/insert-before.test b/lld/test/ELF/linkerscript/insert-before.test
index 922e0859106ff1e..f9611538c013b2b 100644
--- a/lld/test/ELF/linkerscript/insert-before.test
+++ b/lld/test/ELF/linkerscript/insert-before.test
@@ -29,7 +29,7 @@
# CHECK2-NEXT: NULL
# CHECK2-NEXT: .foo.text PROGBITS 000000000020{{.*}} [[#%x,]] 000008 00 AX
# CHECK2-NEXT: .text PROGBITS [[#%x,]] [[#%x,]] 000008 00 AX
-# CHECK2-NEXT: .byte PROGBITS [[#%x,]] [[#%x,]] 000001 00 AX
+# CHECK2-NEXT: .byte PROGBITS [[#%x,]] [[#%x,]] 000001 00 WA
# CHECK2-NEXT: .foo.data PROGBITS [[#%x,]] [[#%x,]] 000008 00 WA
# CHECK2-NEXT: .data PROGBITS [[#%x,]] [[#%x,]] 000008 00 WA
# CHECK2: Type {{.*}} Flg Align
diff --git a/lld/test/ELF/map-file-copy.s b/lld/test/ELF/map-file-copy.s
index 58c0ce981048093..029724418450889 100644
--- a/lld/test/ELF/map-file-copy.s
+++ b/lld/test/ELF/map-file-copy.s
@@ -19,7 +19,7 @@
# CHECK-NEXT: :(.bss.rel.ro)
## Ideally this is displayed as copy@v2.
# CHECK-NEXT: copy{{$}}
-# CHECK-NEXT: .got.plt
+# CHECK-NEXT: .relro_padding
#--- 1.s
.global _start
diff --git a/lld/test/ELF/map-file.s b/lld/test/ELF/map-file.s
index 59931409c7abdb3..0cb20d5a4164bcb 100644
--- a/lld/test/ELF/map-file.s
+++ b/lld/test/ELF/map-file.s
@@ -87,6 +87,8 @@ labs = 0x1AB5
# CHECK-NEXT: 201420 201420 0 1 sharedFunc2
# CHECK-NEXT: 202430 202430 100 8 .dynamic
# CHECK-NEXT: 202430 202430 100 8 :(.dynamic)
+# CHECK-NEXT: 202530 202530 ad0 1 .relro_padding
+# CHECK-NEXT: 202530 202530 0 1 :(.relro_padding)
# CHECK-NEXT: 203530 203530 28 8 .got.plt
# CHECK-NEXT: 203530 203530 28 8 :(.got.plt)
# CHECK-NEXT: 203560 203560 10 16 .bss
@@ -100,8 +102,8 @@ labs = 0x1AB5
# CHECK-NEXT: 0 0 8 1 :(.comment)
# CHECK-NEXT: 0 0 1b0 8 .symtab
# CHECK-NEXT: 0 0 1b0 8 :(.symtab)
-# CHECK-NEXT: 0 0 84 1 .shstrtab
-# CHECK-NEXT: 0 0 84 1 :(.shstrtab)
+# CHECK-NEXT: 0 0 93 1 .shstrtab
+# CHECK-NEXT: 0 0 93 1 :(.shstrtab)
# CHECK-NEXT: 0 0 71 1 .strtab
# CHECK-NEXT: 0 0 71 1 :(.strtab)
diff --git a/lld/test/ELF/partition-notes.s b/lld/test/ELF/partition-notes.s
index 9bc43f2fbf9ee4c..c5ade3a47e05282 100644
--- a/lld/test/ELF/partition-notes.s
+++ b/lld/test/ELF/partition-notes.s
@@ -37,7 +37,7 @@
// CHECK-NEXT: Owner: GNU
// CHECK-NEXT: Data size:
// CHECK-NEXT: Type: NT_GNU_BUILD_ID (unique build ID bitstring)
-// CHECK-NEXT: Build ID: ab81108a3d85b729980356331fddc2bfc4c10177{{$}}
+// CHECK-NEXT: Build ID: d5101cb9d03b7e836ba9b64f5768a0b31980920f{{$}}
// CHECK-NEXT: }
// CHECK-NEXT: }
// CHECK-NEXT: ]
diff --git a/lld/test/ELF/partition-synthetic-sections.s b/lld/test/ELF/partition-synthetic-sections.s
index 2eec08392fe68d3..d38597856e165ef 100644
--- a/lld/test/ELF/partition-synthetic-sections.s
+++ b/lld/test/ELF/partition-synthetic-sections.s
@@ -41,6 +41,7 @@
// PART0-NEXT: .plt PROGBITS
// PART0-NEXT: .init_array INIT_ARRAY {{0*}}[[INIT_ARRAY_ADDR:[^ ]*]]
// CHECK-NEXT: .dynamic DYNAMIC {{0*}}[[DYNAMIC_ADDR:[^ ]*]]
+// PART0-NEXT: .relro_padding NOBITS
// PART0-NEXT: .data PROGBITS 000000000000[[DATA_SEGMENT:.]]178
// PART1-NEXT: .data PROGBITS 000000000000[[DATA_SEGMENT:.]]130
// PART0-NEXT: .got.plt PROGBITS {{0*}}[[GOT_PLT_ADDR:[^ ]*]]
diff --git a/lld/test/ELF/relro-bss.s b/lld/test/ELF/relro-bss.s
index 8fc8167bb51b7dc..f15aff36872df5c 100644
--- a/lld/test/ELF/relro-bss.s
+++ b/lld/test/ELF/relro-bss.s
@@ -10,7 +10,7 @@
# Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
# SEG: LOAD 0x0001c8 0x00000000002011c8 0x00000000002011c8 0x000001 0x000001 R E 0x1000
-# SEG-NEXT: LOAD 0x0001c9 0x00000000002021c9 0x00000000002021c9 0x000001 0x002001 RW 0x1000
+# SEG-NEXT: LOAD 0x0001c9 0x00000000002021c9 0x00000000002021c9 0x000001 0x002e37 RW 0x1000
# SEG-NEXT: LOAD 0x0001ca 0x00000000002051ca 0x00000000002051ca 0x000001 0x000002 RW 0x1000
# SEG-NEXT: GNU_RELRO 0x0001c9 0x00000000002021c9 0x00000000002021c9 0x000001 0x002e37 R 0x1
# SEG-NEXT: GNU_STACK 0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW 0x0
@@ -24,6 +24,7 @@
# [Nr] Name Type Address Off Size
# CHECK: .data.rel.ro PROGBITS 00000000002021c9 0001c9 000001
# CHECK-NEXT: .bss.rel.ro NOBITS 00000000002021ca 0001ca 002000
+# CHECK-NEXT: .relro_padding NOBITS 00000000002041ca 0001ca 000e36
# CHECK-NEXT: .data PROGBITS 00000000002051ca 0001ca 000001
# CHECK-NEXT: .bss NOBITS 00000000002051cb 0001cb 000001
diff --git a/lld/test/ELF/relro-copyrel-bss-script.s b/lld/test/ELF/relro-copyrel-bss-script.s
index 9a947d898a12a1d..4745caae8679039 100644
--- a/lld/test/ELF/relro-copyrel-bss-script.s
+++ b/lld/test/ELF/relro-copyrel-bss-script.s
@@ -28,4 +28,4 @@ _start:
.space 0x2000
// CHECK: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
-// CHECK: GNU_RELRO 0x002150 0x0000000000000150 0x0000000000000150 0x000100 0x000eb0 R 0x1
+// CHECK: GNU_RELRO 0x002150 0x0000000000000150 0x0000000000000150 0x000100 0x000100 R 0x1
diff --git a/lld/test/ELF/relro.s b/lld/test/ELF/relro.s
index 163e257e2956e0a..0ab9665443d1fb3 100644
--- a/lld/test/ELF/relro.s
+++ b/lld/test/ELF/relro.s
@@ -24,8 +24,8 @@
// CHECK-NEXT: GNU_RELRO
// CHECK: Section to Segment mapping:
-// FULLRELRO: 03 .openbsd.randomdata .dynamic .got .got.plt {{$}}
-// PARTRELRO: 03 .openbsd.randomdata .dynamic .got {{$}}
+// FULLRELRO: 03 .openbsd.randomdata .dynamic .got .got.plt .relro_padding {{$}}
+// PARTRELRO: 03 .openbsd.randomdata .dynamic .got .relro_padding {{$}}
// NORELRO-NOT: GNU_RELRO
diff --git a/lld/test/ELF/riscv-section-layout.s b/lld/test/ELF/riscv-section-layout.s
index bb9adf5eef0545b..56ac95d01fdfd61 100644
--- a/lld/test/ELF/riscv-section-layout.s
+++ b/lld/test/ELF/riscv-section-layout.s
@@ -20,6 +20,7 @@
# NOSDATA-NEXT: .tbss
# NOSDATA-NEXT: .dynamic
# NOSDATA-NEXT: .got
+# NOSDATA-NEXT: .relro_padding
# NOSDATA-NEXT: .data PROGBITS [[#%x,DATA:]]
# NOSDATA-NEXT: .bss NOBITS [[#%x,BSS:]]
@@ -36,6 +37,7 @@
# CHECK-NEXT: .tbss
# CHECK-NEXT: .dynamic
# CHECK-NEXT: .got
+# CHECK-NEXT: .relro_padding
# CHECK-NEXT: .data
# CHECK-NEXT: .sdata PROGBITS [[#%x,SDATA:]]
# CHECK-NEXT: .sbss NOBITS [[#%x,SBSS:]]
diff --git a/lld/test/ELF/section-name.s b/lld/test/ELF/section-name.s
index fff3fc14f30246f..819cd9c14d95001 100644
--- a/lld/test/ELF/section-name.s
+++ b/lld/test/ELF/section-name.s
@@ -48,11 +48,12 @@ _start:
// CHECK-NEXT: .tdata 00000001
// CHECK-NEXT: .tbss 00000001
// CHECK-NEXT: .data.rel.ro 00000004
+// CHECK-NEXT: .relro_padding 00000df5
// CHECK-NEXT: .data 00000002
// CHECK-NEXT: .foo.a 00000001
// CHECK-NEXT: .foo 00000001
// CHECK-NEXT: .bss 00000002
// CHECK-NEXT: .comment 00000008
// CHECK-NEXT: .symtab 00000030
-// CHECK-NEXT: .shstrtab 00000075
+// CHECK-NEXT: .shstrtab 00000084
// CHECK-NEXT: .strtab ...
|
MaskRay
changed the title
MaskRay
force-pushed
the
lld-relro
branch
2 times, most recently
from
9a1c225
to
0fa363b
Compare
smithp35
reviewed
Sep 12, 2023
There was a problem hiding this comment.
I've not had a chance to go through the code changes in detail yet.
Can you help me out a bit with what is causing the problem? Is it just that strip is corrupting the output, or is that just one of the problems?
The description says "system-page-size < common-page-size".
What does system-page-size refer to? Do you mean max-page-size? Or is it something like the page size used by the OS? For example: AArch64 max-page-size = 64Kib common-page-size = 4 KiB. An OS may set system-page-size to 4KiB but if I link with max-page-size and common-page-size set to 64 KiB then system-page-size < common-page (and max-page-size).
I mention that because we currently don't permit common-page-size to be larger than max-page-size.
getCommonPageSize()
...
// commonPageSize can't be larger than maxPageSize. if (val > config->maxPageSize)
val = config->maxPageSize;
In the description
in the absence of PHDRS/SECTIONS: ensure the end of PT_GNU_RELRO associated PT_LOAD to a common-page-size boundary
Looks like the sentence is missing a few words on alignment. Perhaps:
ensure the end of the PT_LOAD program loader associated with the PT_GNU_RELRO segment is aligned to a common-page-size boundary.
Thanks for the comments. I used system-page-size to mean the OS's page-size. I have renamed it to "runtime page-size" to improve clarity. Updated the description of the PR: The issue was why [ELF] Align the end of PT_GNU_RELRO to max-page-size instead of common-page-size was incorrect.
Updated. Thanks for the suggestion. |
|
Maybe you want to leave a comment somewhere in the code about what is |
|
Improve comments to RelroPaddingSection. Thanks to rui314 |
smithp35
reviewed
Sep 13, 2023
There was a problem hiding this comment.
Thanks for updating the description, it is clearer what the problem is now.
Is the concern about the padding section that there may be no pattern, or orphan placement might put it in the wrong place? For example in a linker script with DATA_SEGMENT_RELRO_END I could see it being worth inserting the padding section in that case as there would be a clear insertion point.
I agree that most people using linker scripts are embedded users, although another use case is using ld --verbose and editing it then using it with lld. This would have DATA_SEGMENT_RELRO_END.
Are there sufficient differences from GNU ld to make it worth updating the linker script differences doc https://lld.llvm.org/ELF/linker_script.html
lld/ELF/LinkerScript.cpp
Outdated
| if (discardable) { | ||
| sec->markDead(); | ||
| cmd = nullptr; | ||
| } else { | ||
| seenRelro |= sec->relro && sec->type != SHT_NOBITS; |
There was a problem hiding this comment.
Would it be worth restricting this to sec->type != SHT_NOBITS && (sec->flags & SHF_TLS)
There was a problem hiding this comment.
Looks good. Added a new test to relocation-copy-relro.s where .bss.rel.ro is the only RELRO section and we still append the padding.
MaskRay
force-pushed
the
lld-relro
branch
3 times, most recently
from
ab1ee9a
to
8790f8f
Compare
Thanks for the suggestion!
Yes, I wanted to avoid dealing with orphan placement. It turns out that supporting
Added some documentation. |
smithp35
approved these changes
Sep 14, 2023
…ge-size boundary Close llvm#57618: currently we align the end of PT_GNU_RELRO to a common-page-size boundary, but do not align the end of the associated PT_LOAD. This is benign when runtime_page_size >= common-page-size. However, when runtime_page_size < common-page-size, it is possible that `alignUp(end(PT_LOAD), page_size) < alignDown(end(PT_GNU_RELRO), page_size)`. In this case, rtld's mprotect call for PT_GNU_RELRO will apply to unmapped regions and lead to an error, e.g. ``` error while loading shared libraries: cannot apply additional memory protection after relocation: Cannot allocate memory ``` To fix the issue, add a padding section .relro_padding like mold, which is contained in the PT_GNU_RELRO segment and the associated PT_LOAD segment. The section also prevents strip from corrupting PT_LOAD program headers. .relro_padding has the largest `sortRank` among RELRO sections. Therefore, it is naturally placed at the end of `PT_GNU_RELRO` segment in the absence of `PHDRS`/`SECTIONS` commands. In the presence of `SECTIONS` commands, we place .relro_padding immediately before a symbol assignment using DATA_SEGMENT_RELRO_END (see also https://reviews.llvm.org/D124656), if present. DATA_SEGMENT_RELRO_END is changed to align to max-page-size instead of common-page-size. Some edge cases worth mentioning: * ppc64-toc-addis-nop.s: when PHDRS is present, do not append .relro_padding * avoid-empty-program-headers.s: when the only RELRO section is .tbss, it is not part of PT_LOAD segment, therefore we do not append .relro_padding. --- Close llvm#65002: GNU ld from 2.39 onwards aligns the end of PT_GNU_RELRO to a max-page-size boundary (https://sourceware.org/PR28824) so that the last page is protected even if runtime_page_size > common-page-size. In my opinion, losing protection for the last page when the runtime page size is larger than common-page-size is not really an issue. Double mapping a page of up to max-common-page for the protection could cause undesired VM waste. Internally we had users complaining about 2MiB max-page-size applying to shared objects. Therefore, the end of .relro_padding is padded to a common-page-size boundary. Users who are really anxious can set common-page-size to match their runtime page size. --- 17 tests need updating as there are lots of change detectors.
|
This seems to have broken the AMDGPU OpenMP staging buildbot https://lab.llvm.org/staging/#/builders/247/builds/6482 |
dyung
reviewed
Sep 14, 2023
|
We started seeing a test failure: |
We started seeing a test failure: Fixed by 72bbac4 |
ZijunZhaoCCK
pushed a commit
to ZijunZhaoCCK/llvm-project
that referenced
this pull request
Sep 19, 2023
…ge-size boundary (llvm#66042) Close llvm#57618: currently we align the end of PT_GNU_RELRO to a common-page-size boundary, but do not align the end of the associated PT_LOAD. This is benign when runtime_page_size >= common-page-size. However, when runtime_page_size < common-page-size, it is possible that `alignUp(end(PT_LOAD), page_size) < alignDown(end(PT_GNU_RELRO), page_size)`. In this case, rtld's mprotect call for PT_GNU_RELRO will apply to unmapped regions and lead to an error, e.g. ``` error while loading shared libraries: cannot apply additional memory protection after relocation: Cannot allocate memory ``` To fix the issue, add a padding section .relro_padding like mold, which is contained in the PT_GNU_RELRO segment and the associated PT_LOAD segment. The section also prevents strip from corrupting PT_LOAD program headers. .relro_padding has the largest `sortRank` among RELRO sections. Therefore, it is naturally placed at the end of `PT_GNU_RELRO` segment in the absence of `PHDRS`/`SECTIONS` commands. In the presence of `SECTIONS` commands, we place .relro_padding immediately before a symbol assignment using DATA_SEGMENT_RELRO_END (see also https://reviews.llvm.org/D124656), if present. DATA_SEGMENT_RELRO_END is changed to align to max-page-size instead of common-page-size. Some edge cases worth mentioning: * ppc64-toc-addis-nop.s: when PHDRS is present, do not append .relro_padding * avoid-empty-program-headers.s: when the only RELRO section is .tbss, it is not part of PT_LOAD segment, therefore we do not append .relro_padding. --- Close llvm#65002: GNU ld from 2.39 onwards aligns the end of PT_GNU_RELRO to a max-page-size boundary (https://sourceware.org/PR28824) so that the last page is protected even if runtime_page_size > common-page-size. In my opinion, losing protection for the last page when the runtime page size is larger than common-page-size is not really an issue. Double mapping a page of up to max-common-page for the protection could cause undesired VM waste. Internally we had users complaining about 2MiB max-page-size applying to shared objects. Therefore, the end of .relro_padding is padded to a common-page-size boundary. Users who are really anxious can set common-page-size to match their runtime page size. --- 17 tests need updating as there are lots of change detectors.
keszybz
added a commit
to keszybz/systemd
that referenced
this pull request
Mar 14, 2024
Resolves systemd#31637. clang-18 does the section setup differently than older versions. There is a bunch of ordering chagnes, but it also inserts the following: Sections: Idx Name Size VMA LMA File off Algn ... 9 .got 00000000 00000000000283c0 00000000000283c0 000283c0 2**3 CONTENTS, ALLOC, LOAD, DATA 10 .relro_padding 00000c40 00000000000283c0 00000000000283c0 000283c0 2**0 ALLOC 11 .data 00000024 00000000000293c0 00000000000293c0 000283c0 2**4 CONTENTS, ALLOC, LOAD, DATA ... This causes a problem for us, because we try to map the .got to .rodata, and the subsequent .data to .data, and round down the VMA to the nearest page, which causes the PE sections to overlap. llvm/llvm-project#66042 adds .relro_padding to make sure that the RELRO segment is properly write protected and allocated. For our binaries, the .got section is empty, so I think it we can skip it safely, and the .relro_padding section is not useful once .got has been dropped.
keszybz
added a commit
to keszybz/systemd
that referenced
this pull request
Mar 17, 2024
Resolves systemd#31637. lld-18 does the section setup differently than older versions. There is a bunch of ordering chagnes, but it also inserts the following: Sections: Idx Name Size VMA LMA File off Algn ... 9 .got 00000000 00000000000283c0 00000000000283c0 000283c0 2**3 CONTENTS, ALLOC, LOAD, DATA 10 .relro_padding 00000c40 00000000000283c0 00000000000283c0 000283c0 2**0 ALLOC 11 .data 00000024 00000000000293c0 00000000000293c0 000283c0 2**4 CONTENTS, ALLOC, LOAD, DATA ... This causes a problem for us, because we try to map the .got to .rodata, and the subsequent .data to .data, and round down the VMA to the nearest page, which causes the PE sections to overlap. llvm/llvm-project#66042 adds .relro_padding to make sure that the RELRO segment is properly write protected and allocated. For our binaries, the .got section is empty, so we can skip it safely, and the .relro_padding section is not useful once .got has been dropped. To make sure that the code does not use the .got section unexpectedly, raise an error if a (non-empty) .got section is found.
keszybz
added a commit
to keszybz/systemd
that referenced
this pull request
Mar 22, 2024
Resolves systemd#31637. lld-18 does the section setup differently than older versions. There is a bunch of ordering chagnes, but it also inserts the following: Sections: Idx Name Size VMA LMA File off Algn ... 9 .got 00000000 00000000000283c0 00000000000283c0 000283c0 2**3 CONTENTS, ALLOC, LOAD, DATA 10 .relro_padding 00000c40 00000000000283c0 00000000000283c0 000283c0 2**0 ALLOC 11 .data 00000024 00000000000293c0 00000000000293c0 000283c0 2**4 CONTENTS, ALLOC, LOAD, DATA ... This causes a problem for us, because we try to map the .got to .rodata, and the subsequent .data to .data, and round down the VMA to the nearest page, which causes the PE sections to overlap. llvm/llvm-project#66042 adds .relro_padding to make sure that the RELRO segment is properly write protected and allocated. For our binaries, the .got section is empty, so we can skip it safely, and the .relro_padding section is not useful once .got has been dropped. We don't expect .got sections, but they are apparently inserted on i386 and aarch64 builds. Emit a warning until we figure out why they are there.
chunyi-wu
pushed a commit
to chunyi-wu/systemd
that referenced
this pull request
Apr 3, 2024
Resolves systemd#31637. lld-18 does the section setup differently than older versions. There is a bunch of ordering chagnes, but it also inserts the following: Sections: Idx Name Size VMA LMA File off Algn ... 9 .got 00000000 00000000000283c0 00000000000283c0 000283c0 2**3 CONTENTS, ALLOC, LOAD, DATA 10 .relro_padding 00000c40 00000000000283c0 00000000000283c0 000283c0 2**0 ALLOC 11 .data 00000024 00000000000293c0 00000000000293c0 000283c0 2**4 CONTENTS, ALLOC, LOAD, DATA ... This causes a problem for us, because we try to map the .got to .rodata, and the subsequent .data to .data, and round down the VMA to the nearest page, which causes the PE sections to overlap. llvm/llvm-project#66042 adds .relro_padding to make sure that the RELRO segment is properly write protected and allocated. For our binaries, the .got section is empty, so we can skip it safely, and the .relro_padding section is not useful once .got has been dropped. We don't expect .got sections, but they are apparently inserted on i386 and aarch64 builds. Emit a warning until we figure out why they are there.
keszybz
added a commit
to systemd/systemd-stable
that referenced
this pull request
Apr 23, 2024
Resolves systemd/systemd#31637. lld-18 does the section setup differently than older versions. There is a bunch of ordering chagnes, but it also inserts the following: Sections: Idx Name Size VMA LMA File off Algn ... 9 .got 00000000 00000000000283c0 00000000000283c0 000283c0 2**3 CONTENTS, ALLOC, LOAD, DATA 10 .relro_padding 00000c40 00000000000283c0 00000000000283c0 000283c0 2**0 ALLOC 11 .data 00000024 00000000000293c0 00000000000293c0 000283c0 2**4 CONTENTS, ALLOC, LOAD, DATA ... This causes a problem for us, because we try to map the .got to .rodata, and the subsequent .data to .data, and round down the VMA to the nearest page, which causes the PE sections to overlap. llvm/llvm-project#66042 adds .relro_padding to make sure that the RELRO segment is properly write protected and allocated. For our binaries, the .got section is empty, so we can skip it safely, and the .relro_padding section is not useful once .got has been dropped. We don't expect .got sections, but they are apparently inserted on i386 and aarch64 builds. Emit a warning until we figure out why they are there. (cherry picked from commit 6d03e55)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close #57618: currently we align the end of PT_GNU_RELRO to a common-page-size
boundary, but do not align the end of the associated PT_LOAD. This is benign
when runtime_page_size >= common-page-size.
However, when runtime_page_size < common-page-size, it is possible that
alignUp(end(PT_LOAD), page_size) < alignDown(end(PT_GNU_RELRO), page_size).In this case, rtld's mprotect call for PT_GNU_RELRO will apply to unmapped
regions and lead to an error, e.g.
To fix the issue, add a padding section .relro_padding like mold, which
is contained in the PT_GNU_RELRO segment and the associated PT_LOAD
segment. The section also prevents strip from corrupting PT_LOAD program
headers.
.relro_padding has the largest
sortRankamong RELRO sections.Therefore, it is naturally placed at the end of
PT_GNU_RELROsegmentin the absence of
PHDRS/SECTIONScommands.In the presence of
SECTIONScommands, we place .relro_paddingimmediately before a symbol assignment using DATA_SEGMENT_RELRO_END (see
also https://reviews.llvm.org/D124656), if present.
DATA_SEGMENT_RELRO_END is changed to align to max-page-size instead of common-page-size.
Some edge cases worth mentioning:
it is not part of PT_LOAD segment, therefore we do not append .relro_padding.
Close #65002: GNU ld from 2.39 onwards aligns the end of PT_GNU_RELRO to a
max-page-size boundary (https://sourceware.org/PR28824) so that the last page is
protected even if runtime_page_size > common-page-size.
In my opinion, losing protection for the last page when the runtime page size is
larger than common-page-size is not really an issue. Double mapping a page of up
to max-common-page for the protection could cause undesired VM waste. Internally
we had users complaining about 2MiB max-page-size applying to shared objects.
Therefore, the end of .relro_padding is padded to a common-page-size
boundary. Users who are really anxious can set common-page-size to match
their runtime page size.
17 tests need updating as there are lots of change detectors.