ELF: Round p_memsz of the PT_GNU_RELRO program header up to the page size.

[pcc]

Summary

The glibc dynamic loader rounds the size down, so without this the loader
will fail to change the memory protection for the last page.

[emaste]

lld/ELF/Writer.cpp

1452–1453 ↗

(On Diff #82978)

For reference the FreeBSD rtld algorithm is:

obj->relro_page = obj->relocbase + trunc_page(ph->p_vaddr);
obj->relro_size = round_page(ph->p_memsz);

https://svnweb.freebsd.org/base/head/libexec/rtld-elf/rtld.c?annotate=310422#l1334

[pcc]

• Add a note about FreeBSD

lld/ELF/Writer.cpp

1452–1453 ↗

(On Diff #82978)

Thanks. round_page appears to be system-specific but always seems to round up [0]. I've added a note to the comment.

[0] https://github.com/freebsd/freebsd/search?utf8=%E2%9C%93&q=%22define+round_page%22&type=Code

[davide]

LGTM. Out of curiosity, how did you discover this?

[pcc]

In D28267#635130, @davide wrote:

LGTM. Out of curiosity, how did you discover this?

To test my change D28272, I compiled and ran a small test program (essentially the one in figure 2 in the referenced paper). I was surprised to find that even after the copy relocation was moved to relro the program terminated without segfaulting. An strace revealed that the loader wasn't calling mprotect on the relro region at all, which turned out to be because it was smaller than the page size.

lld/trunk/ELF/Writer.cpp

if (First) {
       P.p_memsz = Last->Addr + Last->Size - First->Addr;
       P.p_offset = First->Offset;
       P.p_vaddr = First->Addr;
       if (!P.HasLMA)
         P.p_paddr = First->getLMA();
     }
     if (P.p_type == PT_LOAD)
       P.p_align = Config->MaxPageSize;
-    else if (P.p_type == PT_GNU_RELRO)
+    else if (P.p_type == PT_GNU_RELRO) {
       P.p_align = 1;
+      // The glibc dynamic loader 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 = alignTo(P.p_memsz, Config->MaxPageSize);
+    }
 
     // The TLS pointer goes after PT_TLS. At least glibc will align it,
     // so round up the size to make sure the offsets are correct.
     if (P.p_type == PT_TLS) {
       Out<ELFT>::TlsPhdr = &P;
       if (P.p_memsz)
         P.p_memsz = alignTo(P.p_memsz, P.p_align);
     }

lld/trunk/test/ELF/basic-mips.s

 # CHECK-NEXT:     Alignment: 65536
 # CHECK-NEXT:   }
 # CHECK-NEXT:   ProgramHeader {
 # CHECK-NEXT:     Type: PT_GNU_RELRO (0x6474E552)
 # CHECK-NEXT:     Offset: 0x20000
 # CHECK-NEXT:     VirtualAddress: 0x30000
 # CHECK-NEXT:     PhysicalAddress: 0x30000
 # CHECK-NEXT:     FileSize: 8
-# CHECK-NEXT:     MemSize: 8
+# CHECK-NEXT:     MemSize: 65536
 # CHECK-NEXT:     Flags [ (0x4)
 # CHECK-NEXT:       PF_R (0x4)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Alignment: 1
 # CHECK-NEXT:   }
 # CHECK-NEXT:   ProgramHeader {
 # CHECK-NEXT:    Type: PT_GNU_STACK
 # CHECK-NEXT:    Offset: 0x0

lld/trunk/test/ELF/basic-ppc.s

 // CHECK-NEXT:     Alignment: 4
 // CHECK-NEXT:   }
 // CHECK-NEXT:   ProgramHeader {
 // CHECK-NEXT:     Type: PT_GNU_RELRO (0x6474E552)
 // CHECK-NEXT:     Offset: 0x2000
 // CHECK-NEXT:     VirtualAddress: 0x2000
 // CHECK-NEXT:     PhysicalAddress: 0x2000
 // CHECK-NEXT:     FileSize: 48
-// CHECK-NEXT:     MemSize: 48
+// CHECK-NEXT:     MemSize: 4096
 // CHECK-NEXT:     Flags [ (0x4)
 // CHECK-NEXT:       PF_R (0x4)
 // CHECK-NEXT:     ]
 // CHECK-NEXT:     Alignment: 1
 // CHECK-NEXT:   }
 // CHECK-NEXT:   ProgramHeader {
 // CHECK-NEXT:     Type: PT_GNU_STACK (0x6474E551)
 // CHECK-NEXT:     Offset: 0x0