release-request-a84b3435-75fa-41e8-bd3c-ca0f4cbd5cc5-for-git_oc-m2-release-4352002 snap-temp-L88700000105039240

Change-Id: Ifdeb3942140cc4e9e225e6b24681d251c98aaa98

6 files changed, 49 insertions, 25 deletions

diff --git a/libc/bionic/pthread_internal.cpp b/libc/bionic/pthread_internal.cpp
index abd403bd4..829194cc7 100644
--- a/libc/bionic/pthread_internal.cpp
+++ b/libc/bionic/pthread_internal.cpp
@@ -104,9 +104,13 @@ pthread_internal_t* __pthread_internal_find(pthread_t thread_id) {
   // Check if we're looking for ourselves before acquiring the lock.
   if (thread == __get_thread()) return thread;
 
-  ScopedReadLock locker(&g_thread_list_lock);
-  for (pthread_internal_t* t = g_thread_list; t != nullptr; t = t->next) {
-    if (t == thread) return thread;
+  {
+    // Make sure to release the lock before the abort below. Otherwise,
+    // some apps might deadlock in their own crash handlers (see b/6565627).
+    ScopedReadLock locker(&g_thread_list_lock);
+    for (pthread_internal_t* t = g_thread_list; t != nullptr; t = t->next) {
+      if (t == thread) return thread;
+    }
   }
 
   // Historically we'd return null, but
diff --git a/libdl/libdl_cfi.cpp b/libdl/libdl_cfi.cpp
index 483364fcf..1dd5b21cd 100644
--- a/libdl/libdl_cfi.cpp
+++ b/libdl/libdl_cfi.cpp
@@ -52,7 +52,10 @@ static uint16_t shadow_load(void* p) {
 
 static uintptr_t cfi_check_addr(uint16_t v, void* Ptr) {
   uintptr_t addr = reinterpret_cast<uintptr_t>(Ptr);
-  uintptr_t aligned_addr = align_up(addr, CFIShadow::kShadowAlign);
+  // The aligned range of [0, kShadowAlign) uses a single shadow element, therefore all pointers in
+  // this range must get the same aligned_addr below. This matches CFIShadowWriter::Add; not the
+  // same as align_up().
+  uintptr_t aligned_addr = align_down(addr, CFIShadow::kShadowAlign) + CFIShadow::kShadowAlign;
   uintptr_t p = aligned_addr - (static_cast<uintptr_t>(v - CFIShadow::kRegularShadowMin)
                                 << CFIShadow::kCfiCheckGranularity);
 #ifdef __arm__
diff --git a/tests/Android.bp b/tests/Android.bp
index e8fa5bdf9..29204b558 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -299,12 +299,6 @@ cc_test_library {
             ],
         }
     },
-
-    product_variables: {
-        debuggable: {
-            cppflags: ["-DUSE_LD_CONFIG_FILE"],
-        },
-    },
 }
 
 // -----------------------------------------------------------------------------
@@ -605,12 +599,6 @@ cc_test_host {
     sanitize: {
         never: false,
     },
-
-    product_variables: {
-        debuggable: {
-            cppflags: ["-DUSE_LD_CONFIG_FILE"],
-        },
-    },
 }
 
 subdirs = ["libs"]
diff --git a/tests/cfi_test.cpp b/tests/cfi_test.cpp
index 088dda603..5e2518f07 100644
--- a/tests/cfi_test.cpp
+++ b/tests/cfi_test.cpp
@@ -22,6 +22,10 @@
 #include "gtest_globals.h"
 #include "utils.h"
 
+#if defined(__BIONIC__)
+#include "private/CFIShadow.h"
+#endif
+
 // Private libdl interface.
 extern "C" {
 void __cfi_slowpath(uint64_t CallSiteTypeId, void* Ptr);
@@ -40,15 +44,16 @@ TEST(cfi_test, basic) {
   EXPECT_NE(0U, __cfi_shadow_size());
 
 #define SYM(type, name) auto name = reinterpret_cast<type>(dlsym(handle, #name))
-  SYM(int (*)(), get_count);
+  SYM(size_t (*)(), get_count);
   SYM(uint64_t(*)(), get_last_type_id);
   SYM(void* (*)(), get_last_address);
   SYM(void* (*)(), get_last_diag);
   SYM(void* (*)(), get_global_address);
   SYM(void (*)(uint64_t, void*, void*), __cfi_check);
+  SYM(char*, bss);
 #undef SYM
 
-  int c = get_count();
+  size_t c = get_count();
 
   // CFI check for code inside the DSO. Can't use just any function address - this is only
   // guaranteed to work for code addresses above __cfi_check.
@@ -88,6 +93,14 @@ TEST(cfi_test, basic) {
   EXPECT_DEATH(__cfi_slowpath(46, p), "");
   free(p);
 
+  // Check all the addresses.
+  const size_t bss_size = 1024 * 1024;
+  static_assert(bss_size >= kLibraryAlignment * 2, "test range not big enough");
+  for (size_t i = 0; i < bss_size; ++i) {
+    __cfi_slowpath(47, bss + i);
+    EXPECT_EQ(++c, get_count());
+  }
+
   // Load the same library again.
   void* handle2 = dlopen("libcfi-test.so", RTLD_NOW | RTLD_LOCAL);
   ASSERT_TRUE(handle2 != nullptr) << dlerror();
diff --git a/tests/dl_test.cpp b/tests/dl_test.cpp
index aaf2c3742..d09330dbc 100644
--- a/tests/dl_test.cpp
+++ b/tests/dl_test.cpp
@@ -180,12 +180,24 @@ static void create_ld_config_file(std::string& config_file) {
 }
 #endif
 
-#ifdef USE_LD_CONFIG_FILE
+#if defined(__BIONIC__)
+static bool is_user_build() {
+  std::string build_type = android::base::GetProperty("ro.build.type", "user");
+  if (build_type == "userdebug" || build_type == "eng") {
+    return false;
+  }
+  return true;
+}
+#endif
 
 // _lib1.so and _lib2.so are now searchable by having another namespace 'ns2'
 // whose search paths include the 'ns2/' subdir.
 TEST(dl, exec_with_ld_config_file) {
 #if defined(__BIONIC__)
+  if (is_user_build()) {
+    // LD_CONFIG_FILE is not supported on user build
+    return;
+  }
   std::string helper = get_testlib_root() +
       "/ld_config_test_helper/ld_config_test_helper";
   std::string config_file = get_testlib_root() + "/ld.config.txt";
@@ -204,6 +216,10 @@ TEST(dl, exec_with_ld_config_file) {
 // additional namespaces other than the default namespace.
 TEST(dl, exec_with_ld_config_file_with_ld_preload) {
 #if defined(__BIONIC__)
+  if (is_user_build()) {
+    // LD_CONFIG_FILE is not supported on user build
+    return;
+  }
   std::string helper = get_testlib_root() +
       "/ld_config_test_helper/ld_config_test_helper";
   std::string config_file = get_testlib_root() + "/ld.config.txt";
@@ -218,8 +234,6 @@ TEST(dl, exec_with_ld_config_file_with_ld_preload) {
 #endif
 }
 
-#endif // USE_LD_CONFIG_FILE
-
 // ensures that LD_CONFIG_FILE env var does not work for production builds.
 // The test input is the same as exec_with_ld_config_file, but it must fail in
 // this case.
@@ -230,8 +244,7 @@ TEST(dl, disable_ld_config_file) {
     // This test is only for CTS.
     return;
   }
-  std::string build_type = android::base::GetProperty("ro.build.type", "user");
-  if (build_type == "userdebug" || build_type == "eng") {
+  if (!is_user_build()) {
     // Skip the test for non production devices
     return;
   }
diff --git a/tests/libs/cfi_test_lib.cpp b/tests/libs/cfi_test_lib.cpp
index 959b1020f..9f456d39b 100644
--- a/tests/libs/cfi_test_lib.cpp
+++ b/tests/libs/cfi_test_lib.cpp
@@ -22,13 +22,16 @@
 // present. But it is only used in the bionic loader tests.
 extern "C" __attribute__((weak)) void __cfi_slowpath(uint64_t, void*);
 
-static int g_count;
+static size_t g_count;
 static uint64_t g_last_type_id;
 static void* g_last_address;
 static void* g_last_diag;
 
 extern "C" {
 
+// Make sure the library crosses at least one kLibraryAlignment(=256KB) boundary.
+char bss[1024 * 1024];
+
 // Mock a CFI-enabled library without relying on the compiler.
 __attribute__((aligned(4096))) void __cfi_check(uint64_t CallSiteTypeId, void* TargetAddr,
                                                 void* Diag) {
@@ -38,7 +41,7 @@ __attribute__((aligned(4096))) void __cfi_check(uint64_t CallSiteTypeId, void* T
   g_last_diag = Diag;
 }
 
-int get_count() {
+size_t get_count() {
   return g_count;
 }