Bug 1455662 - Guard against mprotect() failure when manipulating link map r=jchen
authorJames Willcox <snorp@snorp.net>
Wed, 11 Apr 2018 16:49:11 -0500
changeset 471748 3c15a3b3c85a5520f524ad88268774d50ed56b23
parent 471747 e316258ed9aeeed1257b138dbaa1f03899828b84
child 471749 c1b58d542d97346deea3aaaee2929cc803531fbe
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjchen
bugs1455662
milestone61.0a1
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
Bug 1455662 - Guard against mprotect() failure when manipulating link map r=jchen MozReview-Commit-ID: 7orhBmf4j5j
mozglue/linker/ElfLoader.cpp
--- a/mozglue/linker/ElfLoader.cpp
+++ b/mozglue/linker/ElfLoader.cpp
@@ -899,26 +899,38 @@ public:
     length = lastPageEnd - firstPage;
     uintptr_t start = reinterpret_cast<uintptr_t>(firstPage);
     uintptr_t end;
 
     prot = getProt(start, &end);
     if (prot == -1 || (start + length) > end)
       MOZ_CRASH();
 
-    if (prot & PROT_WRITE)
+    if (prot & PROT_WRITE) {
+      success = true;
       return;
+    }
 
     page = firstPage;
-    mprotect(page, length, prot | PROT_WRITE);
+    int ret = mprotect(page, length, prot | PROT_WRITE);
+    success = ret == 0;
+    if (!success) {
+      ERROR("mprotect(%p, %zu, %d) = %d (errno=%d; %s)",
+            page, length, prot | PROT_WRITE, ret,
+            errno, strerror(errno));
+    }
+  }
+
+  bool IsWritable() const {
+    return success;
   }
 
   ~EnsureWritable()
   {
-    if (page != MAP_FAILED) {
+    if (success && page != MAP_FAILED) {
       mprotect(page, length, prot);
 }
   }
 
 private:
   int getProt(uintptr_t addr, uintptr_t *end)
   {
     /* The interesting part of the /proc/self/maps format looks like:
@@ -948,16 +960,17 @@ private:
       return result;
     }
     return -1;
   }
 
   int prot;
   void *page;
   size_t length;
+  bool success;
 };
 
 /**
  * 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
@@ -971,54 +984,73 @@ private:
  * program executable, so the system linker should not be changing
  * r_debug::r_map.
  */
 void
 ElfLoader::DebuggerHelper::Add(ElfLoader::link_map *map)
 {
   if (!dbg->r_brk)
     return;
+
   dbg->r_state = r_debug::RT_ADD;
   dbg->r_brk();
-  map->l_prev = nullptr;
-  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);
+    if (!w.IsWritable()) {
+      dbg->r_state = r_debug::RT_CONSISTENT;
+      dbg->r_brk();
+      return;
+    }
+
+    firstAdded = map;
     dbg->r_map->l_prev = map;
   } else
     dbg->r_map->l_prev = map;
+
+  map->l_prev = nullptr;
+  map->l_next = dbg->r_map;
+
   dbg->r_map = map;
   dbg->r_state = r_debug::RT_CONSISTENT;
   dbg->r_brk();
 }
 
 void
 ElfLoader::DebuggerHelper::Remove(ElfLoader::link_map *map)
 {
   if (!dbg->r_brk)
     return;
+
   dbg->r_state = r_debug::RT_DELETE;
   dbg->r_brk();
+
+  if (map == firstAdded) {
+    /* 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);
+    if (!w.IsWritable()) {
+      dbg->r_state = r_debug::RT_CONSISTENT;
+      dbg->r_brk();
+      return;
+    }
+
+    firstAdded = map->l_prev;
+    map->l_next->l_prev = map->l_prev;
+  } else if (map->l_next) {
+    map->l_next->l_prev = map->l_prev;
+  }
+
   if (dbg->r_map == map)
     dbg->r_map = map->l_next;
   else if (map->l_prev) {
     map->l_prev->l_next = map->l_next;
   }
-  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 if (map->l_next) {
-    map->l_next->l_prev = map->l_prev;
-  }
   dbg->r_state = r_debug::RT_CONSISTENT;
   dbg->r_brk();
 }
 
 #if defined(ANDROID) && defined(__NR_sigaction)
 /* As some system libraries may be calling signal() or sigaction() to
  * set a SIGSEGV handler, effectively breaking MappableSeekableZStream,
  * or worse, restore our SIGSEGV handler with wrong flags (which using