Bug 1455662 - Guard against mprotect() failure when manipulating link map r=jchen

MozReview-Commit-ID: 7orhBmf4j5j

mozglue/linker/ElfLoader.cpp

@@ -904,16 +904,28 @@ public:
     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);
 }
   }
@@ -953,6 +965,7 @@ private:
   int prot;
   void *page;
   size_t length;
+  bool success;
 };
 
 /**
@@ -976,18 +989,28 @@ 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();
@@ -998,22 +1021,31 @@ 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();
 }