Backed out 2 changesets (bug 635961) at developer's request a=backout
authorWes Kocher <wkocher@mozilla.com>
Tue, 25 Jul 2017 17:57:43 -0700
changeset 419659 6af0ca47efabcc081abc4f0e19922e1dbe06d4ab
parent 419658 215c7f7ff02642dce345f19610f0188c789cac57
child 419660 395cdb10f4299bdf41674d68dfb3b64b2056090c
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs635961
milestone56.0a1
backs outc56fa9c1eda0243aef35cbb81f105957fff6dd31
ddda63d5366eca8bec018171271fbb426321eab1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out 2 changesets (bug 635961) at developer's request a=backout Backed out changeset c56fa9c1eda0 (bug 635961) Backed out changeset ddda63d5366e (bug 635961) MozReview-Commit-ID: I6NxBctFn8e
build/unix/elfhack/elfhack.cpp
build/unix/elfhack/elfxx.h
build/unix/elfhack/inject.c
old-configure.in
--- a/build/unix/elfhack/elfhack.cpp
+++ b/build/unix/elfhack/elfhack.cpp
@@ -84,18 +84,18 @@ public:
         shdr.sh_size = rels.size() * shdr.sh_entsize;
     }
 private:
     std::vector<Elf_RelHack> rels;
 };
 
 class ElfRelHackCode_Section: public ElfSection {
 public:
-    ElfRelHackCode_Section(Elf_Shdr &s, Elf &e, unsigned int init, unsigned int mprotect_cb)
-    : ElfSection(s, nullptr, nullptr), parent(e), init(init), mprotect_cb(mprotect_cb) {
+    ElfRelHackCode_Section(Elf_Shdr &s, Elf &e, unsigned int init)
+    : ElfSection(s, nullptr, nullptr), parent(e), init(init) {
         std::string file(rundir);
         file += "/inject/";
         switch (parent.getMachine()) {
         case EM_386:
             file += "x86";
             break;
         case EM_X86_64:
             file += "x86_64";
@@ -120,27 +120,19 @@ public:
         for (ElfSection *section = elf->getSection(1); section != nullptr;
              section = section->getNext()) {
             if (section->getType() == SHT_SYMTAB)
                 symtab = (ElfSymtab_Section *) section;
         }
         if (symtab == nullptr)
             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;
-        std::string symbol = "init";
-        if (!init)
-            symbol += "_noinit";
-        if (relro)
-            symbol += "_relro";
-        Elf_SymValue *sym = symtab->lookup(symbol.c_str());
+        Elf_SymValue *sym = symtab->lookup(init ? "init" : "init_noinit");
         if (!sym)
             throw std::runtime_error("Couldn't find an 'init' symbol in the injected code");
 
         entry_point = sym->value.getValue();
 
         // Get all relevant sections from the injected code object.
         add_code_section(sym->value.getSection());
 
@@ -356,24 +348,16 @@ private:
                 if (strcmp(name, "relhack") == 0) {
                     addr = getNext()->getAddr();
                 } else if (strcmp(name, "elf_header") == 0) {
                     // TODO: change this ungly hack to something better
                     ElfSection *ehdr = parent.getSection(1)->getPrevious()->getPrevious();
                     addr = ehdr->getAddr();
                 } else if (strcmp(name, "original_init") == 0) {
                     addr = init;
-                } else if (relro && strcmp(name, "mprotect_cb") == 0) {
-                    addr = mprotect_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.
-                } else if (relro && strcmp(name, "relro_end") == 0) {
-                    addr = (relro->getAddr() + relro->getMemSize()) & ~(align - 1);
                 } 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
                     addr = 0;
                 } else if (strcmp(name, "") == 0) {
                     // This is for R_ARM_V4BX, until we find something better
                     addr = -1;
                 } else {
@@ -414,20 +398,17 @@ private:
                 throw std::runtime_error("Unsupported relocation type");
             }
         }
     }
 
     Elf *elf, &parent;
     std::vector<ElfSection *> code;
     unsigned int init;
-    unsigned int mprotect_cb;
     int entry_point;
-    ElfSegment *relro;
-    unsigned int align;
 };
 
 unsigned int get_addend(Elf_Rel *rel, Elf *elf) {
     ElfLocation loc(rel->r_offset, elf);
     Elf_Addr addr(loc.getBuffer(), Elf_Addr::size(elf->getClass()), elf->getClass(), elf->getData());
     return addr.value;
 }
 
@@ -517,16 +498,18 @@ template <typename Rel_Type>
 int do_relocation_section(Elf *elf, unsigned int rel_type, unsigned int rel_type2, bool force, bool fill)
 {
     ElfDynamic_Section *dyn = elf->getDynSection();
     if (dyn == nullptr) {
         fprintf(stderr, "Couldn't find SHT_DYNAMIC section\n");
         return -1;
     }
 
+    ElfSegment *relro = elf->getSegmentByType(PT_GNU_RELRO);
+
     ElfRel_Section<Rel_Type> *section = (ElfRel_Section<Rel_Type> *)dyn->getSectionForType(Rel_Type::d_tag);
     if (section == nullptr) {
         fprintf(stderr, "No relocations\n");
         return -1;
     }
     assert(section->getType() == Rel_Type::sh_type);
 
     Elf32_Shdr relhack32_section =
@@ -611,17 +594,19 @@ int do_relocation_section(Elf *elf, unsi
         // Keep track of the relocation associated with the first init_array entry.
         if (init_array && i->r_offset == init_array->getAddr()) {
             if (init_array_reloc) {
                 fprintf(stderr, "Found multiple relocations for the first init_array entry. Skipping\n");
                 return -1;
             }
             new_rels.push_back(*i);
             init_array_reloc = new_rels.size();
-        } else if (!(loc.getSection()->getFlags() & SHF_WRITE) || (ELF32_R_TYPE(i->r_info) != rel_type)) {
+        } else if (!(loc.getSection()->getFlags() & SHF_WRITE) || (ELF32_R_TYPE(i->r_info) != rel_type) ||
+                   (relro && (i->r_offset >= relro->getAddr()) &&
+                   (i->r_offset < relro->getAddr() + relro->getMemSize()))) {
             // Don't pack relocations happening in non writable sections.
             // Our injected code is likely not to be allowed to write there.
             new_rels.push_back(*i);
         } else {
             // TODO: check that i->r_addend == *i->r_offset
             if (i->r_offset == relhack_entry.r_offset + relhack_entry.r_info * entry_sz) {
                 relhack_entry.r_info++;
             } else {
@@ -680,82 +665,20 @@ int do_relocation_section(Elf *elf, unsi
             ElfSymtab_Section *symtab = (ElfSymtab_Section *)section->getLink();
             original_init = symtab->syms[ELF32_R_SYM(rel->r_info)].value.getValue() + addend;
         } else {
             fprintf(stderr, "Unsupported relocation type for DT_INIT_ARRAY's first entry. Skipping\n");
             return -1;
         }
     }
 
-    unsigned int mprotect_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.
-    // To do that, we need to find a reference to the mprotect symbol. In case the library
-    // already has one, we use that, but otherwise, we add the symbol.
-    // Then the injected code needs to be able to call the corresponding function, which
-    // means it needs access to a pointer to it. We get such a pointer by making the linker
-    // apply a relocation for the symbol at an address our code can read.
-    // The problem here is that there is not much relocated space where we can put such a
-    // 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());
-            }
-        }
-
-        // 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.
-        for (ElfSegment *segment = elf->getSegmentByType(PT_LOAD); segment;
-             segment = elf->getSegmentByType(PT_LOAD, segment)) {
-            if (segment->getFlags() & PF_W == 0)
-                continue;
-            size_t ptr_size = Elf_Addr::size(elf->getClass());
-            size_t aligned_mem_end = (segment->getAddr() + segment->getFileSize() + ptr_size - 1) & ~(ptr_size - 1);
-            size_t aligned_file_end = (segment->getAddr() + segment->getMemSize() + ptr_size - 1) & ~(ptr_size - 1);
-            if (aligned_mem_end - aligned_file_end >= Elf_Addr::size(elf->getClass())) {
-                mprotect_cb = rel.r_offset = aligned_file_end;
-                break;
-            }
-        }
-
-        if (mprotect_cb == 0) {
-            fprintf(stderr, "Couldn't find .bss. Skipping\n");
-            return -1;
-        }
-    }
-
     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, original_init, mprotect_cb);
+    ElfRelHackCode_Section *relhackcode = new ElfRelHackCode_Section(relhackcode_section, *elf, original_init);
     relhackcode->insertBefore(section);
     relhack->insertAfter(relhackcode);
     if (section->getOffset() + section->getSize() >= old_end) {
         fprintf(stderr, "No gain. Skipping\n");
         return -1;
     }
 
     // Adjust PT_LOAD segments
--- a/build/unix/elfhack/elfxx.h
+++ b/build/unix/elfhack/elfxx.h
@@ -322,27 +322,18 @@ public:
     unsigned int getSize() { return shdr.sh_size; }
     unsigned int getAddrAlign() { return shdr.sh_addralign; }
     unsigned int getEntSize() { return shdr.sh_entsize; }
     const char *getData() { return data; }
     ElfSection *getLink() { return link; }
     SectionInfo getInfo() { return info; }
 
     void shrink(unsigned int newsize) {
-        if (newsize < shdr.sh_size) {
+        if (newsize < shdr.sh_size)
             shdr.sh_size = newsize;
-            markDirty();
-        }
-    }
-
-    void grow(unsigned int newsize) {
-        if (newsize > shdr.sh_size) {
-            shdr.sh_size = newsize;
-            markDirty();
-        }
     }
 
     unsigned int getOffset();
     int getIndex();
     Elf_Shdr &getShdr();
 
     ElfSection *getNext() { return next; }
     ElfSection *getPrevious() { return previous; }
--- a/build/unix/elfhack/inject.c
+++ b/build/unix/elfhack/inject.c
@@ -1,15 +1,13 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include <stdint.h>
-#include <stdlib.h>
-#include <sys/mman.h>
 #include <elf.h>
 
 /* The Android NDK headers define those */
 #undef Elf_Ehdr
 #undef Elf_Addr
 
 #if defined(__LP64__)
 #define Elf_Ehdr Elf64_Ehdr
@@ -19,20 +17,16 @@
 #define Elf_Addr Elf32_Addr
 #endif
 
 extern __attribute__((visibility("hidden"))) void original_init(int argc, char **argv, char **env);
 
 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"))) char relro_start[];
-extern __attribute__((visibility("hidden"))) char relro_end[];
-
 static inline __attribute__((always_inline))
 void do_relocations(void)
 {
     Elf32_Rel *rel;
     Elf_Addr *ptr, *start;
     for (rel = relhack; rel->r_offset; rel++) {
         start = (Elf_Addr *)((intptr_t)&elf_header + rel->r_offset);
         for (ptr = start; ptr < &start[rel->r_info]; ptr++)
@@ -51,45 +45,8 @@ int init_noinit(int argc, char **argv, c
 int init(int argc, char **argv, char **env)
 {
     do_relocations();
     original_init(argc, argv, env);
     // Ensure there is no tail-call optimization, avoiding the use of the
     // B.W instruction in Thumb for the call above.
     return 0;
 }
-
-static inline __attribute__((always_inline))
-void relro_pre()
-{
-    // 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);
-}
-
-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.
-    mprotect_cb = NULL;
-}
-
-__attribute__((section(".text._init_noinit_relro")))
-int init_noinit_relro(int argc, char **argv, char **env)
-{
-    relro_pre();
-    do_relocations();
-    relro_post();
-    return 0;
-}
-
-__attribute__((section(".text._init_relro")))
-int init_relro(int argc, char **argv, char **env)
-{
-    relro_pre();
-    do_relocations();
-    original_init(argc, argv, env);
-    relro_post();
-    return 0;
-}
--- a/old-configure.in
+++ b/old-configure.in
@@ -3976,16 +3976,57 @@ if test "$USE_ELF_HACK" = 1; then
         esac
         ;;
     *)
         USE_ELF_HACK=
         ;;
     esac
 fi
 
+if test -n "$COMPILE_ENVIRONMENT" -a -n "$USE_ELF_HACK"; then
+    dnl PT_GNU_RELRO segment makes the dynamic linker set a read-only flag on
+    dnl memory addresses it maps to. The result is that by the time elfhack
+    dnl kicks in, it is not possible to apply relocations because of that,
+    dnl thus elfhack effectively skips relocations inside the PT_GNU_RELRO
+    dnl segment. It makes elfhack mostly useless, so considering the problems
+    dnl we have we PT_GNU_RELRO (e.g. bug 664366), and until elfhack can deal
+    dnl with PT_GNU_RELRO segments, it's just simpler to disable elfhack when
+    dnl the linker creates PT_GNU_RELRO segments. However, when we do want
+    dnl elfhack enabled, disable PT_GNU_RELRO instead.
+    AC_CACHE_CHECK([whether linker creates PT_GNU_RELRO segments],
+        LINK_WITH_PT_GNU_RELRO,
+        [echo "int main() {return 0;}" > conftest.${ac_ext}
+         if AC_TRY_COMMAND(${CC-cc} -o conftest${ac_exeext} $LDFLAGS conftest.${ac_ext} $LIBS 1>&2) &&
+            test -s conftest${ac_exeext}; then
+            if ${TOOLCHAIN_PREFIX}readelf -l conftest${ac_exeext} | grep GNU_RELRO > /dev/null; then
+                LINK_WITH_PT_GNU_RELRO=yes
+            else
+                LINK_WITH_PT_GNU_RELRO=no
+            fi
+         else
+             dnl We really don't expect to get here, but just in case
+             AC_ERROR([couldn't compile a simple C file])
+         fi
+         rm -rf conftest*])
+    if test "$LINK_WITH_PT_GNU_RELRO" = yes; then
+        if test "$USE_ELF_HACK" = F; then
+            AC_MSG_CHECKING([for -z norelro option to ld])
+            _SAVE_LDFLAGS=$LDFLAGS
+            LDFLAGS="$LDFLAGS -Wl,-z,norelro"
+            AC_TRY_LINK(,,AC_MSG_RESULT([yes])
+                        [NSPR_LDFLAGS="$NSPR_LDFLAGS -Wl,-z,norelro"],
+                        AC_ERROR([--enable-elf-hack is not compatible with a linker creating a PT_GNU_RELRO segment and that doesn't support the "-z norelro" option.]))
+            USE_ELF_HACK=1
+        else
+            AC_MSG_WARN([Disabling elfhack])
+            USE_ELF_HACK=
+        fi
+    fi
+fi # COMPILE_ENVIRONMENT and others.
+
 dnl ========================================================
 dnl = libstdc++ compatibility hacks
 dnl ========================================================
 
 STDCXX_COMPAT=
 MOZ_ARG_ENABLE_BOOL(stdcxx-compat,
 [  --enable-stdcxx-compat  Enable compatibility with older libstdc++],
     STDCXX_COMPAT=1)