Bug 1470701 - Use run-time page size when changing mapping permissions in elfhack...

When a binary has a PT_GNU_RELRO segment, the elfhack injected code
uses mprotect to add the writable flag to relocated pages before
applying relocations, removing it afterwards. To do so, the elfhack
program uses the location and size of the PT_GNU_RELRO segment, and
adjusts it to be aligned according to the PT_LOAD alignment.

The problem here is that the PT_LOAD alignment doesn't necessarily match
the actual page alignment, and the resulting mprotect may end up not
covering the full extent of what the dynamic linker has protected
read-only according to the PT_GNU_RELRO segment. In turn, this can lead
to a crash on startup when trying to apply relocations to the still
read-only locations.

Practically speaking, this doesn't end up being a problem on x86, where
the PT_LOAD alignment is usually 4096, which happens to be the page
size, but on Debian armhf, it is 64k, while the run time page size can be
4k.

Gbp-Pq: Topic fixes
Gbp-Pq: Name Bug-1470701-Use-run-time-page-size-when-changing-map.patch

diff --git a/build/unix/elfhack/elfhack.cpp b/build/unix/elfhack/elfhack.cpp
index c009f15162522c4ac5a45e8a283ef162f6e9538f..a3321c2f40f4ba408324fec9b0a394c49feae3d3 100644 (file)
--- a/build/unix/elfhack/elfhack.cpp
+++ b/build/unix/elfhack/elfhack.cpp
@@ -90,9 +90,10 @@ private:
 class ElfRelHackCode_Section: public ElfSection {
 public:
     ElfRelHackCode_Section(Elf_Shdr &s, Elf &e, ElfRelHack_Section &relhack_section,
-                           unsigned int init, unsigned int mprotect_cb)
+                           unsigned int init, unsigned int mprotect_cb,
+                           unsigned int sysconf_cb)
     : ElfSection(s, nullptr, nullptr), parent(e), relhack_section(relhack_section),
-      init(init), mprotect_cb(mprotect_cb) {
+      init(init), mprotect_cb(mprotect_cb), sysconf_cb(sysconf_cb) {
         std::string file(rundir);
         file += "/inject/";
         switch (parent.getMachine()) {
@@ -128,7 +129,6 @@ public:
             throw std::runtime_error("Couldn't find a symbol table for the injected code");
 
         relro = parent.getSegmentByType(PT_GNU_RELRO);
-        align = parent.getSegmentByType(PT_LOAD)->getAlign();
 
         // Find the init symbol
         entry_point = -1;
@@ -365,12 +365,12 @@ private:
                     addr = init;
                 } else if (relro && strcmp(name, "mprotect_cb") == 0) {
                     addr = mprotect_cb;
+                } else if (relro && strcmp(name, "sysconf_cb") == 0) {
+                    addr = sysconf_cb;
                 } else if (relro && strcmp(name, "relro_start") == 0) {
-                    // Align relro segment start to the start of the page it starts in.
-                    addr = relro->getAddr() & ~(align - 1);
-                    // Align relro segment end to the start of the page it ends into.
+                    addr = relro->getAddr();
                 } else if (relro && strcmp(name, "relro_end") == 0) {
-                    addr = (relro->getAddr() + relro->getMemSize()) & ~(align - 1);
+                    addr = (relro->getAddr() + relro->getMemSize());
                 } else if (strcmp(name, "_GLOBAL_OFFSET_TABLE_") == 0) {
                     // We actually don't need a GOT, but need it as a reference for
                     // GOTOFF relocations. We'll just use the start of the ELF file
@@ -424,9 +424,9 @@ private:
     std::vector<ElfSection *> code;
     unsigned int init;
     unsigned int mprotect_cb;
+    unsigned int sysconf_cb;
     int entry_point;
     ElfSegment *relro;
-    unsigned int align;
 };
 
 unsigned int get_addend(Elf_Rel *rel, Elf *elf) {
@@ -702,6 +702,7 @@ int do_relocation_section(Elf *elf, unsigned int rel_type, unsigned int rel_type
     }
 
     unsigned int mprotect_cb = 0;
+    unsigned int sysconf_cb = 0;
     // If there is a relro segment, our injected code will run after the linker sets the
     // corresponding pages read-only. We need to make our code change that to read-write
     // before applying relocations, which means it needs to call mprotect.
@@ -714,33 +715,47 @@ int do_relocation_section(Elf *elf, unsigned int rel_type, unsigned int rel_type
     // pointer, so we abuse the bss section temporarily (it will be restored to a null
     // value before any code can actually use it)
     if (elf->getSegmentByType(PT_GNU_RELRO)) {
-        Elf_SymValue *mprotect = symtab->lookup("mprotect", STT(FUNC));
-        if (!mprotect) {
-            symtab->syms.emplace_back();
-            mprotect = &symtab->syms.back();
-            symtab->grow(symtab->syms.size() * symtab->getEntSize());
-            mprotect->name = ((ElfStrtab_Section *)symtab->getLink())->getStr("mprotect");
-            mprotect->info = ELF32_ST_INFO(STB_GLOBAL, STT_FUNC);
-            mprotect->other = STV_DEFAULT;
-            new (&mprotect->value) ElfLocation(nullptr, 0, ElfLocation::ABSOLUTE);
-            mprotect->size = 0;
-            mprotect->defined = false;
-
-            // The DT_VERSYM data (in the .gnu.version section) has the same number of
-            // entries as the symbols table. Since we added one entry there, we need to
-            // add one entry here. Zeroes in the extra data means no version for that
-            // symbol, which is the simplest thing to do.
-            ElfSection *gnu_versym = dyn->getSectionForType(DT_VERSYM);
-            if (gnu_versym) {
-               gnu_versym->grow(gnu_versym->getSize() + gnu_versym->getEntSize());
+        ElfSection *gnu_versym = dyn->getSectionForType(DT_VERSYM);
+        auto lookup = [&symtab, &gnu_versym](const char* symbol) {
+            Elf_SymValue *sym_value = symtab->lookup(symbol, STT(FUNC));
+            if (!sym_value) {
+                symtab->syms.emplace_back();
+                sym_value = &symtab->syms.back();
+                symtab->grow(symtab->syms.size() * symtab->getEntSize());
+                sym_value->name = ((ElfStrtab_Section *)symtab->getLink())->getStr(symbol);
+                sym_value->info = ELF32_ST_INFO(STB_GLOBAL, STT_FUNC);
+                sym_value->other = STV_DEFAULT;
+                new (&sym_value->value) ElfLocation(nullptr, 0, ElfLocation::ABSOLUTE);
+                sym_value->size = 0;
+                sym_value->defined = false;
+
+                // The DT_VERSYM data (in the .gnu.version section) has the same number of
+                // entries as the symbols table. Since we added one entry there, we need to
+                // add one entry here. Zeroes in the extra data means no version for that
+                // symbol, which is the simplest thing to do.
+                if (gnu_versym) {
+                   gnu_versym->grow(gnu_versym->getSize() + gnu_versym->getEntSize());
+                }
             }
-        }
+            return sym_value;
+        };
+
+        Elf_SymValue *mprotect = lookup("mprotect");
+        Elf_SymValue *sysconf = lookup("sysconf");
+
+        // Add relocations for the mprotect and sysconf symbols.
+        auto add_relocation_to = [&new_rels, &symtab, rel_type2](Elf_SymValue *symbol, unsigned int location) {
+            new_rels.emplace_back();
+            Rel_Type &rel = new_rels.back();
+            memset(&rel, 0, sizeof(rel));
+            rel.r_info = ELF32_R_INFO(
+                std::distance(symtab->syms.begin(),
+                              std::vector<Elf_SymValue>::iterator(symbol)),
+                rel_type2);
+            rel.r_offset = location;
+            return location;
+        };
 
-        // Add a relocation for the mprotect symbol.
-        new_rels.emplace_back();
-        Rel_Type &rel = new_rels.back();
-        memset(&rel, 0, sizeof(rel));
-        rel.r_info = ELF32_R_INFO(std::distance(symtab->syms.begin(), std::vector<Elf_SymValue>::iterator(mprotect)), rel_type2);
 
         // Find the beginning of the bss section, and use an aligned location in there
         // for the relocation.
@@ -751,13 +766,14 @@ int do_relocation_section(Elf *elf, unsigned int rel_type, unsigned int rel_type
             size_t ptr_size = Elf_Addr::size(elf->getClass());
             size_t usable_start = (s->getAddr() + ptr_size - 1) & ~(ptr_size - 1);
             size_t usable_end = (s->getAddr() + s->getSize()) & ~(ptr_size - 1);
-            if (usable_end - usable_start >= ptr_size) {
-                mprotect_cb = rel.r_offset = usable_start;
+            if (usable_end - usable_start >= 2 * ptr_size) {
+                mprotect_cb = add_relocation_to(mprotect, usable_start);
+                sysconf_cb = add_relocation_to(sysconf, usable_start + ptr_size);
                 break;
             }
         }
 
-        if (mprotect_cb == 0) {
+        if (mprotect_cb == 0 || sysconf_cb == 0) {
             fprintf(stderr, "Couldn't find .bss. Skipping\n");
             return -1;
         }
@@ -766,7 +782,8 @@ int do_relocation_section(Elf *elf, unsigned int rel_type, unsigned int rel_type
     section->rels.assign(new_rels.begin(), new_rels.end());
     section->shrink(new_rels.size() * section->getEntSize());
 
-    ElfRelHackCode_Section *relhackcode = new ElfRelHackCode_Section(relhackcode_section, *elf, *relhack, original_init, mprotect_cb);
+    ElfRelHackCode_Section *relhackcode = new ElfRelHackCode_Section(
+        relhackcode_section, *elf, *relhack, original_init, mprotect_cb, sysconf_cb);
     // Find the first executable section, and insert the relhack code before
     // that. The relhack data is inserted between .rel.dyn and .rel.plt.
     ElfSection *first_executable = nullptr;

diff --git a/build/unix/elfhack/inject.c b/build/unix/elfhack/inject.c
index affc939755fddb716df3b49f523438342e1c692d..345a73e2bc3c539fbf70aefbe799f22d27d4a99c 100644 (file)
--- a/build/unix/elfhack/inject.c
+++ b/build/unix/elfhack/inject.c
@@ -4,6 +4,7 @@
 
 #include <stdint.h>
 #include <stdlib.h>
+#include <unistd.h>
 #include <sys/mman.h>
 #include <elf.h>
 
@@ -25,6 +26,7 @@ extern __attribute__((visibility("hidden"))) Elf32_Rel relhack[];
 extern __attribute__((visibility("hidden"))) Elf_Ehdr elf_header;
 
 extern __attribute__((visibility("hidden"))) int (*mprotect_cb)(void *addr, size_t len, int prot);
+extern __attribute__((visibility("hidden"))) long (*sysconf_cb)(int name);
 extern __attribute__((visibility("hidden"))) char relro_start[];
 extern __attribute__((visibility("hidden"))) char relro_end[];
 
@@ -58,38 +60,40 @@ int init(int argc, char **argv, char **env)
 }
 
 static inline __attribute__((always_inline))
-void relro_pre()
+void do_relocations_with_relro(void)
 {
+    long page_size = sysconf_cb(_SC_PAGESIZE);
+    uintptr_t aligned_relro_start = ((uintptr_t) relro_start) & ~(page_size - 1);
+    uintptr_t aligned_relro_end = ((uintptr_t) relro_end) & ~(page_size - 1);
     // By the time the injected code runs, the relro segment is read-only. But
     // we want to apply relocations in it, so we set it r/w first. We'll restore
     // it to read-only in relro_post.
-    mprotect_cb(relro_start, relro_end - relro_start, PROT_READ | PROT_WRITE);
-}
+    mprotect_cb((void *)aligned_relro_start,
+                aligned_relro_end - aligned_relro_start,
+                PROT_READ | PROT_WRITE);
 
-static inline __attribute__((always_inline))
-void relro_post()
-{
-    mprotect_cb(relro_start, relro_end - relro_start, PROT_READ);
-    // mprotect_cb is a pointer allocated in .bss, so we need to restore it to
-    // a NULL value.
+    do_relocations();
+
+    mprotect_cb((void *)aligned_relro_start,
+                aligned_relro_end - aligned_relro_start,
+                PROT_READ);
+    // mprotect_cb and sysconf_cb are allocated in .bss, so we need to restore
+    // them to a NULL value.
     mprotect_cb = NULL;
+    sysconf_cb = NULL;
 }
 
 __attribute__((section(".text._init_noinit_relro")))
 int init_noinit_relro(int argc, char **argv, char **env)
 {
-    relro_pre();
-    do_relocations();
-    relro_post();
+    do_relocations_with_relro();
     return 0;
 }
 
 __attribute__((section(".text._init_relro")))
 int init_relro(int argc, char **argv, char **env)
 {
-    relro_pre();
-    do_relocations();
-    relro_post();
+    do_relocations_with_relro();
     original_init(argc, argv, env);
     return 0;
 }

diff --git a/build/unix/elfhack/test.c b/build/unix/elfhack/test.c
index 41e2ae82319b75dd4566319100acf731591db951..8c66c19631d1444f4bfb2654aa085aa9cf48728b 100644 (file)
--- a/build/unix/elfhack/test.c
+++ b/build/unix/elfhack/test.c
@@ -129,6 +129,10 @@ int print_status() {
 __thread int foo;
 __thread long long int bar[512];
 
+/* We need a .bss that can hold at least 2 pointers. The static in
+ * end_test() plus this variable should do. */
+size_t dummy;
+
 void end_test() {
     static size_t count = 0;
     /* Only exit when both constructors have been called */