Bug 1414506 - Use system dl_iterate_phdr for system loaded libraries when we can. r?froydnj draft
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 23 Jan 2018 15:59:18 +0900
changeset 747315 3206c56f47a93331dad4aa408ef16da6a1834052
parent 747314 6d8a03a18ab8f70106530b18ecbc8280a3435140
child 747316 c56199f296afb5c8789bab3e57baa7cf938188f0
push id96871
push userbmo:mh+mozilla@glandium.org
push dateThu, 25 Jan 2018 21:23:22 +0000
reviewersfroydnj
bugs1414506
milestone60.0a1
Bug 1414506 - Use system dl_iterate_phdr for system loaded libraries when we can. r?froydnj When looping through the debugger helper links during our dl_iterate_phdr implementation, we effectively race with other threads dlclose()ing libraries while we're working. We do have a (rather involved) check in place to ensure that elf headers are readable. But it turns out in practice, some dlclose() do happen between the check and the actual read of the elf headers. Unfortunately, we can't lock the system linker while we're looping, so a better approach is to only loop through the libraries we loaded, and rely on the system dl_iterate_phdr to iterate over the (remaining) system libraries. Unfortunately (again), Android versions < 5.0 don't have a system dl_iterate_phdr, so we have to rely on the old iterator when it's not present.
mozglue/linker/ElfLoader.cpp
mozglue/linker/ElfLoader.h
--- a/mozglue/linker/ElfLoader.cpp
+++ b/mozglue/linker/ElfLoader.cpp
@@ -36,16 +36,22 @@ inline int sigaltstack(const stack_t *ss
 #endif /* __ANDROID_API__ */
 #endif /* ANDROID */
 
 #ifdef __ARM_EABI__
 extern "C" MOZ_EXPORT const void *
 __gnu_Unwind_Find_exidx(void *pc, int *pcount) __attribute__((weak));
 #endif
 
+/* Ideally we'd #include <link.h>, but that's a world of pain
+ * Moreover, not all versions of android support it, so we need a weak
+ * reference. */
+extern "C" MOZ_EXPORT int
+dl_iterate_phdr(dl_phdr_cb callback, void *data) __attribute__((weak));
+
 /* Pointer to the PT_DYNAMIC section of the executable or library
  * containing this code. */
 extern "C" Elf::Dyn _DYNAMIC[];
 
 using namespace mozilla;
 
 /**
  * dlfcn.h replacements functions
@@ -195,21 +201,41 @@ DlIteratePhdrHelper::fill_and_call(dl_ph
     }
 
     return callback(&info, sizeof(dl_phdr_info), data);
 }
 
 int
 __wrap_dl_iterate_phdr(dl_phdr_cb callback, void *data)
 {
+  DlIteratePhdrHelper helper;
+  AutoLock lock(&ElfLoader::Singleton.handlesMutex);
+
+  if (dl_iterate_phdr) {
+    for (ElfLoader::LibHandleList::reverse_iterator it =
+	   ElfLoader::Singleton.handles.rbegin();
+	 it < ElfLoader::Singleton.handles.rend(); ++it) {
+      BaseElf* elf = (*it)->AsBaseElf();
+      if (!elf) {
+	continue;
+      }
+      int ret = helper.fill_and_call(callback, (*it)->GetBase(),
+				     (*it)->GetPath(), data);
+      if (ret)
+        return ret;
+    }
+    return dl_iterate_phdr(callback, data);
+  }
+
+  /* For versions of Android that don't support dl_iterate_phdr (< 5.0),
+   * we go through the debugger helper data, which is known to be racy, but
+   * there's not much we can do about this :( . */
   if (!ElfLoader::Singleton.dbg)
     return -1;
 
-  DlIteratePhdrHelper helper;
-
   for (ElfLoader::DebuggerHelper::iterator it = ElfLoader::Singleton.dbg.begin();
        it < ElfLoader::Singleton.dbg.end(); ++it) {
     int ret = helper.fill_and_call(callback, it->l_addr, it->l_name, data);
     if (ret)
       return ret;
   }
   return 0;
 }
--- a/mozglue/linker/ElfLoader.h
+++ b/mozglue/linker/ElfLoader.h
@@ -214,16 +214,17 @@ protected:
   /**
    * Returns the instance, casted as the wanted type. Returns nullptr if
    * that's not the actual type. (short of a better way to do this without
    * RTTI)
    */
   friend class ElfLoader;
   friend class CustomElf;
   friend class SEGVHandler;
+  friend int __wrap_dl_iterate_phdr(dl_phdr_cb callback, void *data);
   virtual BaseElf *AsBaseElf() { return nullptr; }
   virtual SystemElf *AsSystemElf() { return nullptr; }
 
 private:
   mozilla::Atomic<MozRefCountType> directRefCnt;
   char *path;
 
   /* Mappable object keeping the result of GetMappable() */