[ELF] - Fill executable segments with trap instructions.

[grimar]

Summary

LLD currently fills the last page of each executable segment with
trap instructions. It seems to be not enough, see PR36853.

Patch changes logic to fill the all paddings within a segment.
So that any zero areas which exist because of alignment
paddings are also covered.

[ruiu]

This change is a bit worrisome from the performance point of view because it significantly increases the number of bytes we write to mmap'ed file. I believe this is still fine because writing bytes to contiguous memory is extremely fast and the size of the executable part of an executable is relatively small compared to the size of an entire executable, but we need to verify that first.

[ruiu]

When I tried this patch against an internal program of size 2 GiB, the overhead almost 2% (i.e. the link time is 2% longer than before), which is I think not negligible. We need a better way of filling executable segments with trap instructions.

[grimar]

In D44775#1046138, @ruiu wrote:

When I tried this patch against an internal program of size 2 GiB, the overhead almost 2% (i.e. the link time is 2% longer than before), which is I think not negligible. We need a better way of filling executable segments with trap instructions.

Thanks for the information. Indeed I did not expect 2% slowdown. I'll try to implement and benchmark a different approach.

[grimar]

• Reimplemented.

[grimar]

• Included forgotten test case.

[ruiu]

ELF/Arch/X86.cpp

445–446

These instructions won't change, so let's just append 0xcc instructions at end of each array, instead of using memset.

[grimar]

• Addressed review comment.

[ruiu]

ELF/Arch/X86.cpp

446

Please remove ; .align 48 because that doesn't really explain what this code does. (Also we want to align it not to 48 but 16).

466

Ditto

ELF/Arch/X86_64.cpp

503–504

Do this

0xcc, 0xcc, ... // int3
0xcc, 0xcc, ... // int3

so that the comments are aligned vertically.

552

Ditto

[grimar]

ELF/Arch/X86.cpp

446

Actually, we want to align to PltHeaderSize I think, which is 48 here. Because if you align 32 to 16, you'll get
32 instead of 48 and that would not work.

[grimar]

Abandoning in favour of D44943

[ruiu]

ELF/Arch/X86.cpp

446

That's wrong. ".align 48" doesn't mean that the size of this thingy is 48 byte. It just says that the thin that follows this thing is aligned to 48 bytes. So that comment is wrong.

[ruiu]

You shouldn't have dropped this patch because mine doesn't include the code to add 0xcc to end of each PLT entry. We still need that for PLT entries that's larger than one page.

[• espindola]

ELF/Writer.cpp

2193

Why not static?

2221

I wonder if we could write this as we write the sections.

[grimar]

In D44775#1050890, @ruiu wrote:

You shouldn't have dropped this patch because mine doesn't include the code to add 0xcc to end of each PLT entry. We still need that for PLT entries that's larger than one page.

I think we have D44682 for PLT entries posted earlier than this one on review and it do all necessary job for PLT entries + has the test case.

[grimar]

ELF/Arch/X86.cpp

446

Ok, seems I overthinking the meaning of "0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, // 19: int3; .align 16" above then,
thanks for the explanation.

ELF/Arch/X86.cpp

const uint8_t Insn[] = {
       0x0f, 0xae, 0xe8,                         // 14:   lfence
       0xeb, 0xf9,                               // 17:   jmp loop
       0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, // 19:   int3; .align 16
       0x89, 0x0c, 0x24,                         // 20: next: mov %ecx, (%esp)
       0x8b, 0x4c, 0x24, 0x04,                   // 23:   mov 0x4(%esp), %ecx
       0x89, 0x44, 0x24, 0x04,                   // 27:   mov %eax ,0x4(%esp)
       0x89, 0xc8,                               // 2b:   mov %ecx, %eax
       0x59,                                     // 2d:   pop %ecx
       0xc3,                                     // 2e:   ret
+      0xcc                                      // 2f:   int3; .align 48
   };
   memcpy(Buf, Insn, sizeof(Insn));
 
   uint32_t Ebx = InX::Got->getVA() + InX::Got->getSize();
   uint32_t GotPlt = InX::GotPlt->getVA() - Ebx;
   write32le(Buf + 2, GotPlt + 4);
   write32le(Buf + 9, GotPlt + 8);
 }
 
 void RetpolinePic::writePlt(uint8_t *Buf, uint64_t GotPltEntryAddr,
                             uint64_t PltEntryAddr, int32_t Index,
                             unsigned RelOff) const {
   const uint8_t Insn[] = {
-      0x50,                   // pushl %eax
-      0x8b, 0x83, 0, 0, 0, 0, // mov foo@GOT(%ebx), %eax
-      0xe8, 0,    0, 0, 0,    // call plt+0x20
-      0xe9, 0,    0, 0, 0,    // jmp plt+0x12
-      0x68, 0,    0, 0, 0,    // pushl $reloc_offset
-      0xe9, 0,    0, 0, 0,    // jmp plt+0
+      0x50,                           // pushl %eax
+      0x8b, 0x83, 0,    0,    0,   0, // mov foo@GOT(%ebx), %eax
+      0xe8, 0,    0,    0,    0,      // call plt+0x20
+      0xe9, 0,    0,    0,    0,      // jmp plt+0x12
+      0x68, 0,    0,    0,    0,      // pushl $reloc_offset
+      0xe9, 0,    0,    0,    0,      // jmp plt+0
+      0xcc, 0xcc, 0xcc, 0xcc, 0xcc    // int3; .align 32
   };
   memcpy(Buf, Insn, sizeof(Insn));
 
   uint32_t Ebx = InX::Got->getVA() + InX::Got->getSize();
   unsigned Off = getPltEntryOffset(Index);
   write32le(Buf + 3, GotPltEntryAddr - Ebx);
   write32le(Buf + 8, -Off - 12 + 32);
   write32le(Buf + 13, -Off - 17 + 18);
const uint8_t PltData[] = {
       0xcc, 0xcc, 0xcc, 0xcc, 0xcc,    // 18:   int3
       0xcc, 0xcc, 0xcc,                // 1f:   int3; .align 16
       0x89, 0x0c, 0x24,                // 20: next: mov %ecx, (%esp)
       0x8b, 0x4c, 0x24, 0x04,          // 23:   mov 0x4(%esp), %ecx
       0x89, 0x44, 0x24, 0x04,          // 27:   mov %eax ,0x4(%esp)
       0x89, 0xc8,                      // 2b:   mov %ecx, %eax
       0x59,                            // 2d:   pop %ecx
       0xc3,                            // 2e:   ret
+      0xcc                             // 2f:   int3; .align 48
   };
   memcpy(Buf, PltData, sizeof(PltData));
 
   uint32_t GotPlt = InX::GotPlt->getVA();
   write32le(Buf + 2, GotPlt + 4);
   write32le(Buf + 8, GotPlt + 8);
 }
 
 void RetpolineNoPic::writePlt(uint8_t *Buf, uint64_t GotPltEntryAddr,
                               uint64_t PltEntryAddr, int32_t Index,
                               unsigned RelOff) const {
   const uint8_t Insn[] = {
-      0x50,             // 0:  pushl %eax
-      0xa1, 0, 0, 0, 0, // 1:  mov foo_in_GOT, %eax
-      0xe8, 0, 0, 0, 0, // 6:  call plt+0x20
-      0xe9, 0, 0, 0, 0, // b:  jmp plt+0x11
-      0x68, 0, 0, 0, 0, // 10: pushl $reloc_offset
-      0xe9, 0, 0, 0, 0, // 15: jmp plt+0
+      0x50,                              // 0:  pushl %eax
+      0xa1, 0,    0,    0,    0,         // 1:  mov foo_in_GOT, %eax
+      0xe8, 0,    0,    0,    0,         // 6:  call plt+0x20
+      0xe9, 0,    0,    0,    0,         // b:  jmp plt+0x11
+      0x68, 0,    0,    0,    0,         // 10: pushl $reloc_offset
+      0xe9, 0,    0,    0,    0,         // 15: jmp plt+0
+      0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc // 16: int3; .align 32
   };
   memcpy(Buf, Insn, sizeof(Insn));
 
   unsigned Off = getPltEntryOffset(Index);
   write32le(Buf + 2, GotPltEntryAddr);
   write32le(Buf + 7, -Off - 11 + 32);
   write32le(Buf + 12, -Off - 16 + 17);
   write32le(Buf + 17, RelOff);

ELF/Arch/X86_64.cpp

 
 template <class ELFT>
 void Retpoline<ELFT>::writeGotPlt(uint8_t *Buf, const Symbol &S) const {
   write32le(Buf, S.getPltVA() + 17);
 }
 
 template <class ELFT> void Retpoline<ELFT>::writePltHeader(uint8_t *Buf) const {
   const uint8_t Insn[] = {
-      0xff, 0x35, 0,    0,    0,    0,          // 0:    pushq GOTPLT+8(%rip)
-      0x4c, 0x8b, 0x1d, 0,    0,    0,    0,    // 6:    mov GOTPLT+16(%rip), %r11
-      0xe8, 0x0e, 0x00, 0x00, 0x00,             // d:    callq next
-      0xf3, 0x90,                               // 12: loop: pause
-      0x0f, 0xae, 0xe8,                         // 14:   lfence
-      0xeb, 0xf9,                               // 17:   jmp loop
+      0xff, 0x35, 0,    0,    0,    0,       // 0:    pushq GOTPLT+8(%rip)
+      0x4c, 0x8b, 0x1d, 0,    0,    0,    0, // 6:    mov GOTPLT+16(%rip), %r11
+      0xe8, 0x0e, 0x00, 0x00, 0x00,          // d:    callq next
+      0xf3, 0x90,                            // 12: loop: pause
+      0x0f, 0xae, 0xe8,                      // 14:   lfence
+      0xeb, 0xf9,                            // 17:   jmp loop
       0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, // 19:   int3; .align 16
       0x4c, 0x89, 0x1c, 0x24,                   // 20: next: mov %r11, (%rsp)
       0xc3,                                     // 24:   ret
+      0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc,
+      0xcc, 0xcc, 0xcc, 0xcc // 25: int3; .align 48
   };
   memcpy(Buf, Insn, sizeof(Insn));
 
   uint64_t GotPlt = InX::GotPlt->getVA();
   uint64_t Plt = InX::Plt->getVA();
   write32le(Buf + 2, GotPlt - Plt - 6 + 8);
   write32le(Buf + 9, GotPlt - Plt - 13 + 16);
 }
 
 template <class ELFT>
 void Retpoline<ELFT>::writePlt(uint8_t *Buf, uint64_t GotPltEntryAddr,
                                uint64_t PltEntryAddr, int32_t Index,
                                unsigned RelOff) const {
   const uint8_t Insn[] = {
-      0x4c, 0x8b, 0x1d, 0, 0, 0, 0, // 0:  mov foo@GOTPLT(%rip), %r11
-      0xe8, 0,    0,    0, 0,       // 7:  callq plt+0x20
-      0xe9, 0,    0,    0, 0,       // c:  jmp plt+0x12
-      0x68, 0,    0,    0, 0,       // 11: pushq <relocation index>
-      0xe9, 0,    0,    0, 0,       // 16: jmp plt+0
+      0x4c, 0x8b, 0x1d, 0,    0,   0, 0, // 0:  mov foo@GOTPLT(%rip), %r11
+      0xe8, 0,    0,    0,    0,         // 7:  callq plt+0x20
+      0xe9, 0,    0,    0,    0,         // c:  jmp plt+0x12
+      0x68, 0,    0,    0,    0,         // 11: pushq <relocation index>
+      0xe9, 0,    0,    0,    0,         // 16: jmp plt+0
+      0xcc, 0xcc, 0xcc, 0xcc, 0xcc       // 1b: int3; .align 32
   };
   memcpy(Buf, Insn, sizeof(Insn));
 
   uint64_t Off = TargetInfo::getPltEntryOffset(Index);
 
   write32le(Buf + 3, GotPltEntryAddr - PltEntryAddr - 7);
   write32le(Buf + 8, -Off - 12 + 32);
   write32le(Buf + 13, -Off - 17 + 18);
void RetpolineZNow<ELFT>::writePltHeader(uint8_t *Buf) const {
   const uint8_t Insn[] = {
       0xe8, 0x0b, 0x00, 0x00, 0x00, // 0:    call next
       0xf3, 0x90,                   // 5:  loop: pause
       0x0f, 0xae, 0xe8,             // 7:    lfence
       0xeb, 0xf9,                   // a:    jmp loop
       0xcc, 0xcc, 0xcc, 0xcc,       // c:    int3; .align 16
       0x4c, 0x89, 0x1c, 0x24,       // 10: next: mov %r11, (%rsp)
       0xc3,                         // 14:   ret
+      0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc,
+      0xcc, 0xcc, 0xcc, 0xcc, 0xcc // 15:    int3; .align 48
   };
   memcpy(Buf, Insn, sizeof(Insn));
 }
 
 template <class ELFT>
 void RetpolineZNow<ELFT>::writePlt(uint8_t *Buf, uint64_t GotPltEntryAddr,
                                    uint64_t PltEntryAddr, int32_t Index,
                                    unsigned RelOff) const {
   const uint8_t Insn[] = {
-      0x4c, 0x8b, 0x1d, 0, 0, 0, 0, // mov foo@GOTPLT(%rip), %r11
-      0xe9, 0,    0,    0, 0,       // jmp plt+0
+      0x4c, 0x8b, 0x1d, 0,   0, 0, 0, // mov foo@GOTPLT(%rip), %r11
+      0xe9, 0,    0,    0,   0,       // jmp plt+0
+      0xcc, 0xcc, 0xcc, 0xcc          // int3; .align 32
   };
   memcpy(Buf, Insn, sizeof(Insn));
 
   write32le(Buf + 3, GotPltEntryAddr - PltEntryAddr - 7);
   write32le(Buf + 8, -TargetInfo::getPltEntryOffset(Index) - 12);
 }
 
 template <class ELFT> TargetInfo *getTargetInfo() {

ELF/Writer.cpp

 
 template <class ELFT> void Writer<ELFT>::writeSectionsBinary() {
   uint8_t *Buf = Buffer->getBufferStart();
   for (OutputSection *Sec : OutputSections)
     if (Sec->Flags & SHF_ALLOC)
       Sec->writeTo<ELFT>(Buf + Sec->Offset);
 }
 
-static void fillTrap(uint8_t *I, uint8_t *End) {
+static std::vector<OutputSection *> getSectionsInLoad(PhdrEntry *Load) {
+  std::vector<OutputSection *> V;
+  for (OutputSection *Sec : OutputSections)
+    if (Sec->PtLoad == Load)
+      V.push_back(Sec);
+  return V;
+}
+
+void fillTrap(uint8_t *I, uint8_t *End) {
   for (; I + 4 <= End; I += 4)
     memcpy(I, &Target->TrapInstr, 4);
+  memcpy(I, &Target->TrapInstr, End - I);
 }
 
 // Fill the last page of executable segments with trap instructions
 // instead of leaving them as zero. Even though it is not required by any
 // standard, it is in general a good thing to do for security reasons.
 //
 // We'll leave other pages in segments as-is because the rest will be
 // overwritten by output sections.
 template <class ELFT> void Writer<ELFT>::writeTrapInstr() {
   if (Script->HasSectionsCommand)
     return;
 
-  // Fill the last page.
+  // Here we want to fill all paddings inside PT_LOAD with trap instructions.
+  // For that, we want to fill all gaps between sections and two gaps between
+  // the first and last sections and segment boundaries. There is never a gap
+  // between the beginning of the segment and first section, so we ignore it.
   uint8_t *Buf = Buffer->getBufferStart();
-  for (PhdrEntry *P : Phdrs)
-    if (P->p_type == PT_LOAD && (P->p_flags & PF_X))
-      fillTrap(Buf + alignDown(P->p_offset + P->p_filesz, Target->PageSize),
-               Buf + alignTo(P->p_offset + P->p_filesz, Target->PageSize));
+  for (PhdrEntry *P : Phdrs) {
+    if (P->p_type != PT_LOAD || !(P->p_flags & PF_X))
+      continue;
+
+    // Fill gaps between output sections.
+    std::vector<OutputSection *> V = getSectionsInLoad(P);
+    for (size_t I = 0; I < V.size() - 1; ++I)
+      fillTrap(Buf + V[I]->Offset + V[I]->Size, Buf + V[I + 1]->Offset);
+
+    // Fill gap between the last section and end of the segment.
+    fillTrap(Buf + V.back()->Offset + V.back()->Size,
+  }
 
   // Round up the file size of the last segment to the page boundary iff it is
   // an executable segment to ensure that other tools don't accidentally
   // trim the instruction padding (e.g. when stripping the file).
   PhdrEntry *Last = nullptr;
   for (PhdrEntry *P : Phdrs)
     if (P->p_type == PT_LOAD)
       Last = P;

test/ELF/fill-trap2.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: ld.lld %t -o %t2
+# RUN: llvm-readobj -program-headers %t2 | FileCheck %s
+# RUN: od -Ax -t x1 -N16 -j0x1000 %t2 | FileCheck %s -check-prefix=FILL
+
+# CHECK: ProgramHeader {
+# CHECK:   Type: PT_LOAD
+# CHECK:   Offset: 0x1000
+# CHECK-NEXT:   VirtualAddress:
+# CHECK-NEXT:   PhysicalAddress:
+# CHECK-NEXT:   FileSize: 8192
+# CHECK-NEXT:   MemSize:
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     PF_R
+# CHECK-NEXT:     PF_X
+# CHECK-NEXT:   ]
+
+# FILL: 001000 90 cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
+
+nop
+
+.section .foo,"ax"
+.align 16
+nop
+.zero 0x1000