Bug 808121 - Ensure the pointers we change in the r_debug data are writable, which they aren't with upcoming Android system linker. r=nfroyd

mozglue/linker/ElfLoader.cpp

@@ -4,6 +4,7 @@
 
 #include <cstring>
 #include <cstdlib>
+#include <cstdio>
 #include <dlfcn.h>
 #include <unistd.h>
 #include <algorithm>
@@ -95,11 +96,11 @@ __wrap_dladdr(void *addr, Dl_info *info)
 int
 __wrap_dl_iterate_phdr(dl_phdr_cb callback, void *data)
 {
-  if (ElfLoader::Singleton.dbg == NULL)
+  if (ElfLoader::Singleton.dbg)
     return -1;
 
-  for (ElfLoader::r_debug::iterator it = ElfLoader::Singleton.dbg->begin();
-       it < ElfLoader::Singleton.dbg->end(); ++it) {
+  for (ElfLoader::DebuggerHelper::iterator it = ElfLoader::Singleton.dbg.begin();
+       it < ElfLoader::Singleton.dbg.end(); ++it) {
     dl_phdr_info info;
     info.dlpi_addr = reinterpret_cast<Elf::Addr>(it->l_addr);
     info.dlpi_name = it->l_name;
@@ -317,7 +318,7 @@ ElfLoader::Register(LibHandle *handle)
 {
   handles.push_back(handle);
   if (dbg && !handle->IsSystemElf())
-    dbg->Add(static_cast<CustomElf *>(handle));
+    dbg.Add(static_cast<CustomElf *>(handle));
 }
 
 void
@@ -328,7 +329,7 @@ ElfLoader::Forget(LibHandle *handle)
     debug("ElfLoader::Forget(%p [\"%s\"])", reinterpret_cast<void *>(handle),
                                             handle->GetPath());
     if (dbg && !handle->IsSystemElf())
-      dbg->Remove(static_cast<CustomElf *>(handle));
+      dbg.Remove(static_cast<CustomElf *>(handle));
     handles.erase(it);
   } else {
     debug("ElfLoader::Forget(%p [\"%s\"]): Handle not found",
@@ -433,8 +434,7 @@ ElfLoader::DestructorCaller::Call()
   }
 }
 
-void
-ElfLoader::InitDebugger()
+ElfLoader::DebuggerHelper::DebuggerHelper(): dbg(NULL)
 {
   /* Find ELF auxiliary vectors.
    *
@@ -577,6 +577,68 @@ ElfLoader::InitDebugger()
   debug("DT_DEBUG points at %p", dbg);
 }
 
+/**
+ * Helper class to ensure the given pointer is writable within the scope of
+ * an instance. Permissions to the memory page where the pointer lies are
+ * restored to their original value when the instance is destroyed.
+ */
+class EnsureWritable
+{
+public:
+  template <typename T>
+  EnsureWritable(T *&ptr)
+  {
+    prot = getProt((uintptr_t) &ptr);
+    if (prot == -1)
+      MOZ_CRASH();
+    /* Pointers are aligned such that their value can't be spanning across
+     * 2 pages. */
+    page = (void*)((uintptr_t) &ptr & PAGE_MASK);
+    if (!(prot & PROT_WRITE))
+      mprotect(page, PAGE_SIZE, prot | PROT_WRITE);
+  }
+
+  ~EnsureWritable()
+  {
+    if (!(prot & PROT_WRITE))
+      mprotect(page, PAGE_SIZE, prot);
+  }
+
+private:
+  int getProt(uintptr_t addr)
+  {
+    /* The interesting part of the /proc/self/maps format looks like:
+     * startAddr-endAddr rwxp */
+    int result = 0;
+    AutoCloseFILE f(fopen("/proc/self/maps", "r"));
+    while (f) {
+      unsigned long long startAddr, endAddr;
+      char perms[5];
+      if (fscanf(f, "%llx-%llx %4s %*1024[^\n] ", &startAddr, &endAddr, perms) != 3)
+        return -1;
+      if (addr < startAddr || addr >= endAddr)
+        continue;
+      if (perms[0] == 'r')
+        result |= PROT_READ;
+      else if (perms[0] != '-')
+        return -1;
+      if (perms[1] == 'w')
+        result |= PROT_WRITE;
+      else if (perms[1] != '-')
+        return -1;
+      if (perms[2] == 'x')
+        result |= PROT_EXEC;
+      else if (perms[2] != '-')
+        return -1;
+      return result;
+    }
+    return -1;
+  }
+
+  int prot;
+  void *page;
+};
+
 /**
  * The system linker maintains a doubly linked list of library it loads
  * for use by the debugger. Unfortunately, it also uses the list pointers
@@ -594,34 +656,48 @@ ElfLoader::InitDebugger()
  * r_debug::r_map.
  */
 void
-ElfLoader::r_debug::Add(ElfLoader::link_map *map)
+ElfLoader::DebuggerHelper::Add(ElfLoader::link_map *map)
 {
-  if (!r_brk)
+  if (!dbg->r_brk)
     return;
-  r_state = RT_ADD;
-  r_brk();
+  dbg->r_state = r_debug::RT_ADD;
+  dbg->r_brk();
   map->l_prev = NULL;
-  map->l_next = r_map;
-  r_map->l_prev = map;
-  r_map = map;
-  r_state = RT_CONSISTENT;
-  r_brk();
+  map->l_next = dbg->r_map;
+  if (!firstAdded) {
+    firstAdded = map;
+    /* When adding a library for the first time, r_map points to data
+     * handled by the system linker, and that data may be read-only */
+    EnsureWritable w(dbg->r_map->l_prev);
+    dbg->r_map->l_prev = map;
+  } else
+    dbg->r_map->l_prev = map;
+  dbg->r_map = map;
+  dbg->r_state = r_debug::RT_CONSISTENT;
+  dbg->r_brk();
 }
 
 void
-ElfLoader::r_debug::Remove(ElfLoader::link_map *map)
+ElfLoader::DebuggerHelper::Remove(ElfLoader::link_map *map)
 {
-  if (!r_brk)
+  if (!dbg->r_brk)
     return;
-  r_state = RT_DELETE;
-  r_brk();
-  if (r_map == map)
-    r_map = map->l_next;
+  dbg->r_state = r_debug::RT_DELETE;
+  dbg->r_brk();
+  if (dbg->r_map == map)
+    dbg->r_map = map->l_next;
   else
     map->l_prev->l_next = map->l_next;
-  map->l_next->l_prev = map->l_prev;
-  r_state = RT_CONSISTENT;
-  r_brk();
+  if (map == firstAdded) {
+    firstAdded = map->l_prev;
+    /* When removing the first added library, its l_next is going to be
+     * data handled by the system linker, and that data may be read-only */
+    EnsureWritable w(map->l_next->l_prev);
+    map->l_next->l_prev = map->l_prev;
+  } else
+    map->l_next->l_prev = map->l_prev;
+  dbg->r_state = r_debug::RT_CONSISTENT;
+  dbg->r_brk();
 }
 
 SEGVHandler::SEGVHandler()

mozglue/linker/ElfLoader.h

@@ -286,7 +286,6 @@ protected:
   const char *lastError;
 
 private:
-  ElfLoader() { InitDebugger(); }
   ~ElfLoader();
 
   /* Bookkeeping */
@@ -368,7 +367,7 @@ private:
   ZipCollection zips;
 
   /* Forward declaration, see further below */
-  class r_debug;
+  class DebuggerHelper;
 public:
   /* Loaded object descriptor for the debugger interface below*/
   struct link_map {
@@ -380,7 +379,7 @@ public:
     const void *l_ld;
 
   private:
-    friend class ElfLoader::r_debug;
+    friend class ElfLoader::DebuggerHelper;
     /* Double linked list of loaded objects. */
     link_map *l_next, *l_prev;
   };
@@ -388,9 +387,40 @@ public:
 private:
   /* Data structure used by the linker to give details about shared objects it
    * loaded to debuggers. This is normally defined in link.h, but Android
-   * headers lack this file. This also gives the opportunity to make it C++. */
-  class r_debug {
+   * headers lack this file. */
+  struct r_debug {
+    /* Version number of the protocol. */
+    int r_version;
+
+    /* Head of the linked list of loaded objects. */
+    link_map *r_map;
+
+    /* Function to be called when updates to the linked list of loaded objects
+     * are going to occur. The function is to be called before and after
+     * changes. */
+    void (*r_brk)(void);
+
+    /* Indicates to the debugger what state the linked list of loaded objects
+     * is in when the function above is called. */
+    enum {
+      RT_CONSISTENT, /* Changes are complete */
+      RT_ADD,        /* Beginning to add a new object */
+      RT_DELETE      /* Beginning to remove an object */
+    } r_state;
+  };
+
+  /* Helper class used to integrate libraries loaded by this linker in
+   * r_debug */
+  class DebuggerHelper
+  {
   public:
+    DebuggerHelper();
+
+    operator bool()
+    {
+      return dbg;
+    }
+
     /* Make the debugger aware of a new loaded object */
     void Add(link_map *map);
 
@@ -416,10 +446,10 @@ private:
       {
         if (other.item == NULL)
           return item ? true : false;
-        MOZ_NOT_REACHED("r_debug::iterator::operator< called with something else than r_debug::end()");
+        MOZ_NOT_REACHED("DebuggerHelper::iterator::operator< called with something else than DebuggerHelper::end()");
       }
     protected:
-      friend class r_debug;
+      friend class DebuggerHelper;
       iterator(const link_map *item): item(item) { }
 
     private:
@@ -428,7 +458,7 @@ private:
 
     iterator begin() const
     {
-      return iterator(r_map);
+      return iterator(dbg ? dbg->r_map : NULL);
     }
 
     iterator end() const
@@ -437,32 +467,11 @@ private:
     }
 
   private:
-    /* Version number of the protocol. */
-    int r_version;
-
-    /* Head of the linked list of loaded objects. */
-    struct link_map *r_map;
-
-    /* Function to be called when updates to the linked list of loaded objects
-     * are going to occur. The function is to be called before and after
-     * changes. */
-    void (*r_brk)(void);
-
-    /* Indicates to the debugger what state the linked list of loaded objects
-     * is in when the function above is called. */
-    enum {
-      RT_CONSISTENT, /* Changes are complete */
-      RT_ADD,        /* Beginning to add a new object */
-      RT_DELETE      /* Beginning to remove an object */
-    } r_state;
+    r_debug *dbg;
+    link_map *firstAdded;
   };
   friend int __wrap_dl_iterate_phdr(dl_phdr_cb callback, void *data);
-  r_debug *dbg;
-
-  /**
-   * Initializes the pointer to the debugger data structure.
-   */
-  void InitDebugger();
+  DebuggerHelper dbg;
 };
 
 #endif /* ElfLoader_h */

mozglue/linker/Utils.h

@@ -90,6 +90,16 @@ struct AutoCloseFDTraits
 };
 typedef mozilla::Scoped<AutoCloseFDTraits> AutoCloseFD;
 
+/**
+ * AutoCloseFILE is a RAII wrapper for POSIX streams
+ */
+struct AutoCloseFILETraits
+{
+  typedef FILE *type;
+  static FILE *empty() { return NULL; }
+  static void release(FILE *f) { fclose(f); }
+};
+typedef mozilla::Scoped<AutoCloseFILETraits> AutoCloseFILE;
 
 /**
  * MappedPtr is a RAII wrapper for mmap()ed memory. It can be used as