loader: fix dlopen performance regression caused by fix for CVE-2017-0670 am: d6b2586...

am: 2542cfdb44

Change-Id: Ie9bce8dfd0722e50026a0aa5d1d81c70725fcd28

diff --git a/libc/tzcode/localtime.c b/libc/tzcode/localtime.c
index f370e87..3f87743 100644 (file)
--- a/libc/tzcode/localtime.c
+++ b/libc/tzcode/localtime.c
@@ -368,7 +368,7 @@ union local_storage {
   } u;
 };
 
-static int __bionic_open_tzdata(const char*);
+static int __bionic_open_tzdata(const char*, int32_t*);
 
 /* Load tz data from the file named NAME into *SP.  Read extended
    format if DOEXTEND.  Use *LSP for temporary storage.  Return 0 on
@@ -396,8 +396,9 @@ tzloadbody(char const *name, struct state *sp, bool doextend,
                  return EINVAL;
        }
 
-#if defined(__ANDROID__)
-       fid = __bionic_open_tzdata(name);
+#if defined(__BIONIC__)
+       int32_t entry_length;
+       fid = __bionic_open_tzdata(name, &entry_length);
 #else
        if (name[0] == ':')
                ++name;
@@ -423,7 +424,11 @@ tzloadbody(char const *name, struct state *sp, bool doextend,
        if (fid < 0)
          return errno;
 
+#if defined(__BIONIC__)
+       nread = read(fid, up->buf, entry_length);
+#else
        nread = read(fid, up->buf, sizeof up->buf);
+#endif
        if (nread < tzheadsize) {
          int err = nread < 0 ? errno : EINVAL;
          close(fid);
@@ -2324,7 +2329,8 @@ time(time_t *p)
 #include <arpa/inet.h> // For ntohl(3).
 
 static int __bionic_open_tzdata_path(const char* path_prefix_variable, const char* path_suffix,
-                                     const char* olson_id) {
+                                     const char* olson_id,
+                                     int32_t* entry_length) {
   const char* path_prefix = getenv(path_prefix_variable);
   if (path_prefix == NULL) {
     fprintf(stderr, "%s: %s not set!\n", __FUNCTION__, path_prefix_variable);
@@ -2422,6 +2428,7 @@ static int __bionic_open_tzdata_path(const char* path_prefix_variable, const cha
 
     if (strcmp(this_id, olson_id) == 0) {
       specific_zone_offset = ntohl(entry->start) + ntohl(header.data_offset);
+      *entry_length = ntohl(entry->length);
       break;
     }
 
@@ -2449,10 +2456,12 @@ static int __bionic_open_tzdata_path(const char* path_prefix_variable, const cha
   return fd;
 }
 
-static int __bionic_open_tzdata(const char* olson_id) {
-  int fd = __bionic_open_tzdata_path("ANDROID_DATA", "/misc/zoneinfo/current/tzdata", olson_id);
+static int __bionic_open_tzdata(const char* olson_id, int32_t* entry_length) {
+  int fd = __bionic_open_tzdata_path("ANDROID_DATA", "/misc/zoneinfo/current/tzdata",
+                                     olson_id, entry_length);
   if (fd < 0) {
-    fd = __bionic_open_tzdata_path("ANDROID_ROOT", "/usr/share/zoneinfo/tzdata", olson_id);
+    fd = __bionic_open_tzdata_path("ANDROID_ROOT", "/usr/share/zoneinfo/tzdata",
+                                   olson_id, entry_length);
     if (fd == -2) {
       // The first thing that 'recovery' does is try to format the current time. It doesn't have
       // any tzdata available, so we must not abort here --- doing so breaks the recovery image!

diff --git a/libc/zoneinfo/tzdata b/libc/zoneinfo/tzdata
index 538e663..57bff7b 100644 (file)
Binary files a/libc/zoneinfo/tzdata and b/libc/zoneinfo/tzdata differ

diff --git a/linker/linker.cpp b/linker/linker.cpp
index 0ab8bd2..a043b85 100644 (file)
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -4156,9 +4156,14 @@ static void init_default_namespace() {
     g_default_ld_paths = kDefaultLdPaths;
   }
 
+  char real_path[PATH_MAX];
   std::vector<std::string> ld_default_paths;
   for (size_t i = 0; g_default_ld_paths[i] != nullptr; ++i) {
-    ld_default_paths.push_back(g_default_ld_paths[i]);
+    if (realpath(g_default_ld_paths[i], real_path) != nullptr) {
+      ld_default_paths.push_back(real_path);
+    } else {
+      ld_default_paths.push_back(g_default_ld_paths[i]);
+    }
   }
 
   g_default_namespace.set_default_library_paths(std::move(ld_default_paths));

diff --git a/linker/linker_phdr.cpp b/linker/linker_phdr.cpp
index 3b60460..9ed612f 100644 (file)
--- a/linker/linker_phdr.cpp
+++ b/linker/linker_phdr.cpp
@@ -525,9 +525,18 @@ bool ElfReader::LoadSegments() {
     }
 
     if (file_length != 0) {
+      int prot = PFLAGS_TO_PROT(phdr->p_flags);
+      // W + E PT_LOAD segments are not allowed.
+      if ((prot & (PROT_EXEC | PROT_WRITE)) == (PROT_EXEC | PROT_WRITE)) {
+        DL_WARN("\"%s\": has W+E (writable and executable) load segments. "
+                "This is a security risk shared libraries with W+E load segments "
+                "will not be supported in a future Android release. "
+                "Please fix the library.", name_.c_str());
+      }
+
       void* seg_addr = mmap64(reinterpret_cast<void*>(seg_page_start),
                             file_length,
-                            PFLAGS_TO_PROT(phdr->p_flags),
+                            prot,
                             MAP_FIXED|MAP_PRIVATE,
                             fd_,
                             file_offset_ + file_page_start);

diff --git a/tests/dlext_test.cpp b/tests/dlext_test.cpp
index ee61d5f..5501c8e 100644 (file)
--- a/tests/dlext_test.cpp
+++ b/tests/dlext_test.cpp
@@ -628,7 +628,9 @@ TEST(dlext, ns_smoke) {
 
   ASSERT_FALSE(android_init_namespaces(path.c_str(), nullptr));
   ASSERT_STREQ("android_init_namespaces failed: error initializing public namespace: "
-               "\"libnstest_public.so\" was not found in the default namespace", dlerror());
+               "a library with soname \"libnstest_public.so\" was not found in the "
+               "default namespace",
+               dlerror());
 
   const std::string lib_path = std::string(getenv("ANDROID_DATA")) + NATIVE_TESTS_PATH;
 

diff --git a/tests/pty_test.cpp b/tests/pty_test.cpp
index a371434..74415d5 100644 (file)
--- a/tests/pty_test.cpp
+++ b/tests/pty_test.cpp
@@ -72,17 +72,18 @@ TEST(pty, forkpty) {
 }
 
 struct PtyReader_28979140_Arg {
- int slave_fd;
- uint32_t data_count;
- bool finished;
- std::atomic<bool> matched;
+  int main_cpu_id;
+  int slave_fd;
+  uint32_t data_count;
+  bool finished;
+  std::atomic<bool> matched;
 };
 
 static void PtyReader_28979140(PtyReader_28979140_Arg* arg) {
   arg->finished = false;
   cpu_set_t cpus;
   ASSERT_EQ(0, sched_getaffinity(0, sizeof(cpu_set_t), &cpus));
-  CPU_CLR(0, &cpus);
+  CPU_CLR(arg->main_cpu_id, &cpus);
   ASSERT_EQ(0, sched_setaffinity(0, sizeof(cpu_set_t), &cpus));
 
   uint32_t counter = 0;
@@ -105,7 +106,9 @@ static void PtyReader_28979140(PtyReader_28979140_Arg* arg) {
 TEST(pty, bug_28979140) {
   // This test is to test a kernel bug, which uses a lock free ring-buffer to
   // pass data through a raw pty, but missing necessary memory barriers.
-  if (sysconf(_SC_NPROCESSORS_ONLN) == 1) {
+  cpu_set_t cpus;
+  ASSERT_EQ(0, sched_getaffinity(0, sizeof(cpu_set_t), &cpus));
+  if (CPU_COUNT(&cpus) < 2) {
     GTEST_LOG_(INFO) << "This test tests bug happens only on multiprocessors.";
     return;
   }
@@ -120,9 +123,20 @@ TEST(pty, bug_28979140) {
   cfmakeraw(&tattr);
   ASSERT_EQ(0, tcsetattr(slave, TCSADRAIN, &tattr));
 
-  // 2. Create thread for slave reader.
-  pthread_t thread;
+  // 2. Make master thread and slave thread running on different cpus:
+  // master thread uses first available cpu, and slave thread uses other cpus.
   PtyReader_28979140_Arg arg;
+  arg.main_cpu_id = -1;
+  for (int i = 0; i < CPU_SETSIZE; i++) {
+    if (CPU_ISSET(i, &cpus)) {
+      arg.main_cpu_id = i;
+      break;
+    }
+  }
+  ASSERT_GE(arg.main_cpu_id, 0);
+
+  // 3. Create thread for slave reader.
+  pthread_t thread;
   arg.slave_fd = slave;
   arg.data_count = TEST_DATA_COUNT;
   arg.matched = true;
@@ -130,11 +144,8 @@ TEST(pty, bug_28979140) {
                               reinterpret_cast<void*(*)(void*)>(PtyReader_28979140),
                               &arg));
 
-  // 3. Make master thread and slave thread running on different cpus:
-  // master thread uses cpu 0, and slave thread uses other cpus.
-  cpu_set_t cpus;
   CPU_ZERO(&cpus);
-  CPU_SET(0, &cpus);
+  CPU_SET(arg.main_cpu_id, &cpus);
   ASSERT_EQ(0, sched_setaffinity(0, sizeof(cpu_set_t), &cpus));
 
   // 4. Send data to slave.