am 6db8f5bb: am d30116cf: Merge "Keep the dynamic linker\'s soinfo pools mostly read-only."

* commit '6db8f5bb75bb79e0b4873e6d293aa25e2c9f090b':
  Keep the dynamic linker's soinfo pools mostly read-only.

linker/debugger.cpp

@@ -32,7 +32,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
-#include <stdbool.h>
 #include <signal.h>
 #include <sys/prctl.h>
 #include <errno.h>

linker/dlfcn.cpp

@@ -57,13 +57,11 @@ const char* dlerror() {
 
 void* dlopen(const char* filename, int flag) {
   ScopedPthreadMutexLocker locker(&gDlMutex);
-  soinfo* result = find_library(filename);
+  soinfo* result = do_dlopen(filename);
   if (result == NULL) {
     __bionic_format_dlerror("dlopen failed", linker_get_error());
     return NULL;
   }
-  soinfo_call_constructors(result);
-  result->refcount++;
   return result;
 }
 
@@ -139,7 +137,7 @@ int dladdr(const void* addr, Dl_info* info) {
 
 int dlclose(void* handle) {
   ScopedPthreadMutexLocker locker(&gDlMutex);
-  return soinfo_unload((soinfo*) handle);
+  return do_dlclose(reinterpret_cast<soinfo*>(handle));
 }
 
 #if defined(ANDROID_ARM_LINKER)
@@ -236,7 +234,8 @@ soinfo libdl_info = {
 
     refcount: 0,
     { l_addr: 0, l_name: 0, l_ld: 0, l_next: 0, l_prev: 0, },
-    constructors_called: 0, load_bias: 0,
+    constructors_called: false,
+    load_bias: 0,
     has_text_relocations: false,
     has_DT_SYMBOLIC: true,
 };

linker/linker.cpp

@@ -31,7 +31,6 @@
 #include <fcntl.h>
 #include <linux/auxvec.h>
 #include <pthread.h>
-#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -74,7 +73,6 @@
  *   and NOEXEC
  */
 
-
 static bool soinfo_link_image(soinfo* si);
 
 // We can't use malloc(3) in the dynamic linker. We use a linked list of anonymous
@@ -293,6 +291,14 @@ static bool ensure_free_list_non_empty() {
   return true;
 }
 
+static void set_soinfo_pool_protection(int protection) {
+  for (soinfo_pool_t* p = gSoInfoPools; p != NULL; p = p->next) {
+    if (mprotect(p, sizeof(*p), protection) == -1) {
+      abort(); // Can't happen.
+    }
+  }
+}
+
 static soinfo* soinfo_alloc(const char* name) {
   if (strlen(name) >= SOINFO_NAME_LEN) {
     DL_ERR("library name \"%s\" too long", name);
@@ -833,11 +839,9 @@ static soinfo* load_library(const char* name) {
     return si.release();
 }
 
-static soinfo *
-init_library(soinfo *si)
-{
-    /* At this point we know that whatever is loaded @ base is a valid ELF
-     * shared library whose segments are properly mapped in. */
+static soinfo* init_library(soinfo* si) {
+  // At this point we know that whatever is loaded @ base is a valid ELF
+  // shared library whose segments are properly mapped in.
   TRACE("[ %5d init_library base=0x%08x sz=0x%08x name='%s') ]\n",
         pid, si->base, si->size, si->name);
 
@@ -868,54 +872,60 @@ static soinfo *find_loaded_library(const char *name)
     return NULL;
 }
 
-soinfo *find_library(const char *name)
-{
-    soinfo *si;
-
-    if (name == NULL)
+static soinfo* find_library_internal(const char* name) {
+  if (name == NULL) {
     return somain;
+  }
 
-    si = find_loaded_library(name);
+  soinfo* si = find_loaded_library(name);
   if (si != NULL) {
     if (si->flags & FLAG_ERROR) {
       DL_ERR("\"%s\" failed to load previously", name);
       return NULL;
     }
-        if(si->flags & FLAG_LINKED) return si;
+    if (si->flags & FLAG_LINKED) {
+      return si;
+    }
     DL_ERR("OOPS: recursive link to \"%s\"", si->name);
     return NULL;
   }
 
   TRACE("[ %5d '%s' has not been loaded yet.  Locating...]\n", pid, name);
   si = load_library(name);
-    if (si == NULL)
-        return NULL;
-    return init_library(si);
+  if (si != NULL) {
+    si = init_library(si);
   }
 
-static void call_destructors(soinfo *si);
+  return si;
+}
 
-int soinfo_unload(soinfo* si) {
+static soinfo* find_library(const char* name) {
+  soinfo* si = find_library_internal(name);
+  if (si != NULL) {
+    si->refcount++;
+  }
+  return si;
+}
+
+static int soinfo_unload(soinfo* si) {
   if (si->refcount == 1) {
     TRACE("%5d unloading '%s'\n", pid, si->name);
-        call_destructors(si);
+    si->CallDestructors();
 
     for (unsigned* d = si->dynamic; *d; d += 2) {
       if (d[0] == DT_NEEDED) {
         soinfo* lsi = find_loaded_library(si->strtab + d[1]);
-                if (lsi) {
-                    TRACE("%5d %s needs to unload %s\n", pid,
-                          si->name, lsi->name);
+        if (lsi != NULL) {
+          TRACE("%5d %s needs to unload %s\n", pid, si->name, lsi->name);
           soinfo_unload(lsi);
         } else {
           // TODO: should we return -1 in this case?
-                    DL_ERR("\"%s\": could not unload dependent library",
-                           si->name);
+          DL_ERR("\"%s\": could not unload dependent library", si->name);
         }
       }
     }
 
-        munmap((char *)si->base, si->size);
+    munmap(reinterpret_cast<void*>(si->base), si->size);
     notify_gdb_of_unload(si);
     soinfo_free(si);
     si->refcount = 0;
@@ -927,6 +937,23 @@ int soinfo_unload(soinfo* si) {
   return 0;
 }
 
+soinfo* do_dlopen(const char* name) {
+  set_soinfo_pool_protection(PROT_READ | PROT_WRITE);
+  soinfo* si = find_library(name);
+  if (si != NULL) {
+    si->CallConstructors();
+  }
+  set_soinfo_pool_protection(PROT_READ);
+  return si;
+}
+
+int do_dlclose(soinfo* si) {
+  set_soinfo_pool_protection(PROT_READ | PROT_WRITE);
+  int result = soinfo_unload(si);
+  set_soinfo_pool_protection(PROT_READ);
+  return result;
+}
+
 /* TODO: don't use unsigned for addrs below. It works, but is not
  * ideal. They should probably be either uint32_t, Elf32_Addr, or unsigned
  * long.
@@ -1270,106 +1297,89 @@ static int mips_relocate_got(soinfo* si, soinfo* needed[]) {
  *
  *   DT_FINI_ARRAY must be parsed in reverse order.
  */
-
-static void call_array(unsigned *ctor, int count, int reverse)
-{
-    int n, inc = 1;
-
-    if (reverse) {
-        ctor += (count-1);
-        inc   = -1;
+void soinfo::CallArray(const char* array_name UNUSED, unsigned* array, int count, bool reverse) {
+  if (array == NULL) {
+    return;
   }
 
-    for(n = count; n > 0; n--) {
-        TRACE("[ %5d Looking at %s *0x%08x == 0x%08x ]\n", pid,
-              reverse ? "dtor" : "ctor",
-              (unsigned)ctor, (unsigned)*ctor);
-        void (*func)() = (void (*)()) *ctor;
-        ctor += inc;
-        if(((int) func == 0) || ((int) func == -1)) continue;
-        TRACE("[ %5d Calling func @ 0x%08x ]\n", pid, (unsigned)func);
+  int step = 1;
+  if (reverse) {
+    array += (count-1);
+    step = -1;
+  }
+
+  TRACE("[ %5d Calling %s @ %p [%d] for '%s' ]\n", pid, array_name, array, count, name);
+
+  for (int n = count; n > 0; n--) {
+    TRACE("[ %5d Looking at %s[%d] *%p == 0x%08x ]\n", pid, array_name, n, array, *array);
+    void (*func)() = (void (*)()) *array;
+    array += step;
+    if (((int) func == 0) || ((int) func == -1)) {
+      continue;
+    }
+    TRACE("[ %5d Calling func @ %p ]\n", pid, func);
     func();
   }
+
+  TRACE("[ %5d Done calling %s for '%s' ]\n", pid, array_name, name);
 }
 
-static void soinfo_call_preinit_constructors(soinfo *si)
-{
-  TRACE("[ %5d Calling preinit_array @ 0x%08x [%d] for '%s' ]\n",
-      pid, (unsigned)si->preinit_array, si->preinit_array_count,
-      si->name);
-  call_array(si->preinit_array, si->preinit_array_count, 0);
-  TRACE("[ %5d Done calling preinit_array for '%s' ]\n", pid, si->name);
-}
-
-void soinfo_call_constructors(soinfo *si)
-{
-    if (si->constructors_called)
+void soinfo::CallFunction(const char* function_name UNUSED, void (*function)()) {
+  if (function == NULL) {
     return;
+  }
 
-    // Set this before actually calling the constructors, otherwise it doesn't
-    // protect against recursive constructor calls. One simple example of
-    // constructor recursion is the libc debug malloc, which is implemented in
-    // libc_malloc_debug_leak.so:
+  TRACE("[ %5d Calling %s @ %p for '%s' ]\n", pid, function_name, function, name);
+  function();
+  TRACE("[ %5d Done calling %s for '%s' ]\n", pid, function_name, name);
+}
+
+void soinfo::CallPreInitConstructors() {
+  CallArray("DT_PREINIT_ARRAY", preinit_array, preinit_array_count, false);
+}
+
+void soinfo::CallConstructors() {
+  if (constructors_called) {
+    return;
+  }
+
+  // We set constructors_called before actually calling the constructors, otherwise it doesn't
+  // protect against recursive constructor calls. One simple example of constructor recursion
+  // is the libc debug malloc, which is implemented in libc_malloc_debug_leak.so:
   // 1. The program depends on libc, so libc's constructor is called here.
   // 2. The libc constructor calls dlopen() to load libc_malloc_debug_leak.so.
-    // 3. dlopen() calls soinfo_call_constructors() with the newly created
+  // 3. dlopen() calls the constructors on the newly created
   //    soinfo for libc_malloc_debug_leak.so.
-    // 4. The debug so depends on libc, so soinfo_call_constructors() is
+  // 4. The debug .so depends on libc, so CallConstructors is
   //    called again with the libc soinfo. If it doesn't trigger the early-
   //    out above, the libc constructor will be called again (recursively!).
-    si->constructors_called = 1;
+  constructors_called = true;
 
-    if (!(si->flags & FLAG_EXE) && si->preinit_array) {
-      DL_ERR("shared library \"%s\" has a preinit_array table @ 0x%08x. "
-          "This is INVALID.", si->name, (unsigned) si->preinit_array);
+  if (!(flags & FLAG_EXE) && preinit_array) {
+    DL_ERR("shared library \"%s\" has a preinit_array table @ %p", name, preinit_array);
+    return;
   }
 
-    if (si->dynamic) {
-        unsigned *d;
-        for(d = si->dynamic; *d; d += 2) {
+  if (dynamic) {
+    for (unsigned* d = dynamic; *d; d += 2) {
       if (d[0] == DT_NEEDED) {
-                soinfo* lsi = find_loaded_library(si->strtab + d[1]);
-                if (!lsi) {
-                    DL_ERR("\"%s\": could not initialize dependent library",
-                           si->name);
+        soinfo* lsi = find_loaded_library(strtab + d[1]);
+        if (lsi == NULL) {
+          DL_ERR("\"%s\": could not initialize dependent library", name);
         } else {
-                    soinfo_call_constructors(lsi);
+          lsi->CallConstructors();
         }
       }
     }
   }
 
-    if (si->init_func) {
-        TRACE("[ %5d Calling init_func @ 0x%08x for '%s' ]\n", pid,
-              (unsigned)si->init_func, si->name);
-        si->init_func();
-        TRACE("[ %5d Done calling init_func for '%s' ]\n", pid, si->name);
+  CallFunction("DT_INIT", init_func);
+  CallArray("DT_INIT_ARRAY", init_array, init_array_count, false);
 }
 
-    if (si->init_array) {
-        TRACE("[ %5d Calling init_array @ 0x%08x [%d] for '%s' ]\n", pid,
-              (unsigned)si->init_array, si->init_array_count, si->name);
-        call_array(si->init_array, si->init_array_count, 0);
-        TRACE("[ %5d Done calling init_array for '%s' ]\n", pid, si->name);
-    }
-
-}
-
-static void call_destructors(soinfo *si)
-{
-    if (si->fini_array) {
-        TRACE("[ %5d Calling fini_array @ 0x%08x [%d] for '%s' ]\n", pid,
-              (unsigned)si->fini_array, si->fini_array_count, si->name);
-        call_array(si->fini_array, si->fini_array_count, 1);
-        TRACE("[ %5d Done calling fini_array for '%s' ]\n", pid, si->name);
-    }
-
-    if (si->fini_func) {
-        TRACE("[ %5d Calling fini_func @ 0x%08x for '%s' ]\n", pid,
-              (unsigned)si->fini_func, si->name);
-        si->fini_func();
-        TRACE("[ %5d Done calling fini_func for '%s' ]\n", pid, si->name);
-    }
+void soinfo::CallDestructors() {
+  CallArray("DT_FINI_ARRAY", fini_array, fini_array_count, true);
+  CallFunction("DT_FINI", fini_func);
 }
 
 /* Force any of the closed stdin, stdout and stderr to be associated with
@@ -1633,17 +1643,15 @@ static bool soinfo_link_image(soinfo* si) {
 
     /* if this is the main executable, then load all of the preloads now */
     if (si->flags & FLAG_EXE) {
-        int i;
         memset(preloads, 0, sizeof(preloads));
-        for(i = 0; gLdPreloadNames[i] != NULL; i++) {
+        for (size_t i = 0; gLdPreloadNames[i] != NULL; i++) {
             soinfo* lsi = find_library(gLdPreloadNames[i]);
-            if(lsi == 0) {
+            if (lsi == NULL) {
                 strlcpy(tmp_err_buf, linker_get_error(), sizeof(tmp_err_buf));
                 DL_ERR("could not load library \"%s\" needed by \"%s\"; caused by %s",
                        gLdPreloadNames[i], si->name, tmp_err_buf);
                 return false;
             }
-            lsi->refcount++;
             preloads[i] = lsi;
         }
     }
@@ -1655,14 +1663,13 @@ static bool soinfo_link_image(soinfo* si) {
         if (d[0] == DT_NEEDED) {
             DEBUG("%5d %s needs %s\n", pid, si->name, si->strtab + d[1]);
             soinfo* lsi = find_library(si->strtab + d[1]);
-            if(lsi == 0) {
+            if (lsi == NULL) {
                 strlcpy(tmp_err_buf, linker_get_error(), sizeof(tmp_err_buf));
                 DL_ERR("could not load library \"%s\" needed by \"%s\"; caused by %s",
                        si->strtab + d[1], si->name, tmp_err_buf);
                 return false;
             }
             *pneeded++ = lsi;
-            lsi->refcount++;
         }
     }
     *pneeded = NULL;
@@ -1910,10 +1917,10 @@ static unsigned __linker_init_post_relocation(unsigned **elfdata, unsigned linke
         exit(EXIT_FAILURE);
     }
 
-    soinfo_call_preinit_constructors(si);
+    si->CallPreInitConstructors();
 
     for (size_t i = 0; preloads[i] != NULL; ++i) {
-        soinfo_call_constructors(preloads[i]);
+        preloads[i]->CallConstructors();
     }
 
     /*After the link_image, the si->base is initialized.
@@ -1922,7 +1929,7 @@ static unsigned __linker_init_post_relocation(unsigned **elfdata, unsigned linke
      *for some arch like x86 could work correctly within so exe.
      */
     map->l_addr = si->base;
-    soinfo_call_constructors(si);
+    si->CallConstructors();
 
 #if TIMING
     gettimeofday(&t1,NULL);
@@ -2056,6 +2063,8 @@ extern "C" unsigned __linker_init(unsigned **elfdata) {
     // the main part of the linker now.
     unsigned start_address = __linker_init_post_relocation(elfdata, linker_addr);
 
+    set_soinfo_pool_protection(PROT_READ);
+
     // Return the address that the calling assembly stub should jump to.
     return start_address;
 }

linker/linker.h

@@ -46,12 +46,9 @@
 // itself at the start of a page.
 #define PAGE_END(x)    PAGE_START((x) + (PAGE_SIZE-1))
 
-void debugger_init();
+// Magic shared structures that GDB knows about.
 
-/* magic shared structures that GDB knows about */
-
-struct link_map
-{
+struct link_map {
   uintptr_t l_addr;
   char * l_name;
   uintptr_t l_ld;
@@ -66,8 +63,7 @@ enum {
   RT_DELETE
 };
 
-struct r_debug
-{
+struct r_debug {
   int32_t r_version;
   struct link_map* r_map;
   void (*r_brk)(void);
@@ -124,34 +120,42 @@ struct soinfo {
   unsigned* fini_array;
   unsigned fini_array_count;
 
-    void (*init_func)(void);
-    void (*fini_func)(void);
+  void (*init_func)();
+  void (*fini_func)();
 
 #if defined(ANDROID_ARM_LINKER)
-    /* ARM EABI section used for stack unwinding. */
+  // ARM EABI section used for stack unwinding.
   unsigned* ARM_exidx;
   unsigned ARM_exidx_count;
 #elif defined(ANDROID_MIPS_LINKER)
 #if 0
-     /* not yet */
+  // Not yet.
   unsigned* mips_pltgot
 #endif
   unsigned mips_symtabno;
   unsigned mips_local_gotno;
   unsigned mips_gotsym;
-#endif /* ANDROID_*_LINKER */
+#endif
 
   unsigned refcount;
   struct link_map linkmap;
 
-    int constructors_called;
+  bool constructors_called;
 
-    /* When you read a virtual address from the ELF file, add this
-     * value to get the corresponding address in the process' address space */
+  // When you read a virtual address from the ELF file, add this
+  // value to get the corresponding address in the process' address space.
   Elf32_Addr load_bias;
 
   bool has_text_relocations;
   bool has_DT_SYMBOLIC;
+
+  void CallConstructors();
+  void CallDestructors();
+  void CallPreInitConstructors();
+
+ private:
+  void CallArray(const char* array_name, unsigned* array, int count, bool reverse);
+  void CallFunction(const char* function_name, void (*function)());
 };
 
 extern soinfo libdl_info;
@@ -203,16 +207,17 @@ extern soinfo libdl_info;
 #define DT_PREINIT_ARRAYSZ 33
 #endif
 
-soinfo *find_library(const char *name);
+soinfo* do_dlopen(const char* name);
+int do_dlclose(soinfo* si);
+
 Elf32_Sym* lookup(const char* name, soinfo** found, soinfo* start);
 soinfo* find_containing_library(const void* addr);
-const char *linker_get_error(void);
+const char* linker_get_error();
 
-int soinfo_unload(soinfo* si);
 Elf32_Sym* soinfo_find_symbol(soinfo* si, const void* addr);
 Elf32_Sym* soinfo_lookup(soinfo* si, const char* name);
-void soinfo_call_constructors(soinfo *si);
 
+void debugger_init();
 extern "C" void notify_gdb_of_libraries();
 
 #endif