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
authorMike Hommey <mh+mozilla@glandium.org>
Wed, 07 Nov 2012 08:02:53 +0100
changeset 120404 e587aa26326e603d0b282e266d3737b9d37ad677
parent 120403 351a7925bc5d4b676e214726021e4a80a2cd89ab
child 120412 70c55e9a3ef6259b0541cee399945befe6d35e19
push id1997
push userakeybl@mozilla.com
push dateMon, 07 Jan 2013 21:25:26 +0000
treeherdermozilla-beta@4baf45cdcf21 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs808121
milestone19.0a1
first release with
nightly linux32
e587aa26326e / 19.0a1 / 20121107030713 / files
nightly linux64
e587aa26326e / 19.0a1 / 20121107030713 / files
nightly mac
e587aa26326e / 19.0a1 / 20121107030713 / files
nightly win32
e587aa26326e / 19.0a1 / 20121107030713 / files
nightly win64
e587aa26326e / 19.0a1 / 20121107030713 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
mozglue/linker/ElfLoader.h
mozglue/linker/Utils.h
--- a/mozglue/linker/ElfLoader.cpp
+++ b/mozglue/linker/ElfLoader.cpp
@@ -1,14 +1,15 @@
 /* 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 <cstring>
 #include <cstdlib>
+#include <cstdio>
 #include <dlfcn.h>
 #include <unistd.h>
 #include <algorithm>
 #include <fcntl.h>
 #include "ElfLoader.h"
 #include "CustomElf.h"
 #include "Mappable.h"
 #include "Logging.h"
@@ -90,21 +91,21 @@ int
     return 0;
   info->dli_fname = handle->GetPath();
   return 1;
 }
 
 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;
     info.dlpi_phdr = NULL;
     info.dlpi_phnum = 0;
 
     // Assuming l_addr points to Elf headers (in most cases, this is true),
     // get the Phdr location from there.
@@ -312,28 +313,28 @@ ElfLoader::GetHandleByPtr(void *addr)
   return NULL;
 }
 
 void
 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
 ElfLoader::Forget(LibHandle *handle)
 {
   LibHandleList::iterator it = std::find(handles.begin(), handles.end(), handle);
   if (it != handles.end()) {
     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",
           reinterpret_cast<void *>(handle), handle->GetPath());
   }
 }
 
 ElfLoader::~ElfLoader()
@@ -428,18 +429,17 @@ ElfLoader::DestructorCaller::Call()
   if (destructor) {
     debug("ElfLoader::DestructorCaller::Call(%p, %p, %p)",
           FunctionPtr(destructor), object, dso_handle);
     destructor(object);
     destructor = NULL;
   }
 }
 
-void
-ElfLoader::InitDebugger()
+ElfLoader::DebuggerHelper::DebuggerHelper(): dbg(NULL)
 {
   /* Find ELF auxiliary vectors.
    *
    * The kernel stores the following data on the stack when starting a
    * program:
    *   argc
    *   argv[0] (pointer into argv strings defined below)
    *   argv[1] (likewise)
@@ -573,60 +573,136 @@ ElfLoader::InitDebugger()
       dbg = reinterpret_cast<r_debug *>(dyn->d_un.d_ptr);
       break;
     }
   }
   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
  * in a lot of operations and adding our data in the list is likely to
  * trigger crashes when the linker tries to use data we don't provide or
  * that fall off the amount data we allocated. Fortunately, the linker only
  * traverses the list forward and accesses the head of the list from a
  * private pointer instead of using the value in the r_debug structure.
  * This means we can safely add members at the beginning of the list.
  * Unfortunately, gdb checks the coherency of l_prev values, so we have
  * to adjust the l_prev value for the first element the system linker
  * knows about. Fortunately, it doesn't use l_prev, and the first element
  * is not ever going to be released before our elements, since it is the
  * program executable, so the system linker should not be changing
  * 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()
 {
   /* Setup an alternative stack if the already existing one is not big
    * enough, or if there is none. */
   if (sigaltstack(NULL, &oldStack) == -1 || !oldStack.ss_sp ||
       oldStack.ss_size < stackSize) {
--- a/mozglue/linker/ElfLoader.h
+++ b/mozglue/linker/ElfLoader.h
@@ -281,17 +281,16 @@ protected:
   /* Last error. Used for dlerror() */
   friend class SystemElf;
   friend const char *__wrap_dlerror(void);
   friend void *__wrap_dlsym(void *handle, const char *symbol);
   friend int __wrap_dlclose(void *handle);
   const char *lastError;
 
 private:
-  ElfLoader() { InitDebugger(); }
   ~ElfLoader();
 
   /* Bookkeeping */
   typedef std::vector<LibHandle *> LibHandleList;
   LibHandleList handles;
 
 protected:
   friend class CustomElf;
@@ -363,39 +362,70 @@ protected:
 private:
   /* Keep track of all registered destructors */
   std::vector<DestructorCaller> destructors;
 
   /* Keep track of Zips used for library loading */
   ZipCollection zips;
 
   /* Forward declaration, see further below */
-  class r_debug;
+  class DebuggerHelper;
 public:
   /* Loaded object descriptor for the debugger interface below*/
   struct link_map {
     /* Base address of the loaded object. */
     const void *l_addr;
     /* File name */
     const char *l_name;
     /* Address of the PT_DYNAMIC segment. */
     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;
   };
 
 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);
 
     /* Make the debugger aware of the unloading of an object */
     void Remove(link_map *map);
 
     /* Iterates over all link_maps */
     class iterator
@@ -411,58 +441,37 @@ private:
         item = item->l_next;
         return *item;
       }
 
       bool operator<(const iterator &other) const
       {
         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:
       const link_map *item;
     };
 
     iterator begin() const
     {
-      return iterator(r_map);
+      return iterator(dbg ? dbg->r_map : NULL);
     }
 
     iterator end() const
     {
       return iterator(NULL);
     }
 
   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 */
--- a/mozglue/linker/Utils.h
+++ b/mozglue/linker/Utils.h
@@ -85,16 +85,26 @@ typedef le_to_cpu<le_uint16> le_uint32;
 struct AutoCloseFDTraits
 {
   typedef int type;
   static int empty() { return -1; }
   static void release(int fd) { close(fd); }
 };
 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
  * a simple void * or unsigned char *.
  *
  * It is defined as a derivative of a template that allows to use a
  * different unmapping strategy.
  */