Avoid loading external libraries from ARTs internal linker namespace

(reland).

dlopen() calls in ART will use its own linker namespace
(com_android_art). That's appropriate for internal libraries in the
APEX, but not when ART loads libraries on behalf of external requests.
In those cases we should instead use android_dlopen_ext to load them
from the system namespace, i.e. the one that searches /system/lib(64).

The linker config has been using allow_all_shared_libs, so any loads
from com_android_art fall back to the system namespace, and hence
dlopen() usually works regardless which namespace it ought to use.
However we want to drop allow_all_shared_libs, so we need to figure out
which dlopen's should use which namespace:

1. Several ART libraries are loaded on-demand, e.g. libart-compiler and
   libart-disassembler. There are also those going through the runtime
   plugin system, like libperfetto_hprofd and heapprofd_client_api. All
   these are internal or at least statically known (so we can provide
   links for them in the linker config), and should continue to use
   dlopen from the ART namespace.

2. libnativeloader loads the preloadable public libraries from
   system/etc/public.libraries.txt, and should use the system namespace
   for that.

3. libnativebridge loads the native bridge implementation specified
   through the command line (or ultimately the system property
   ro.dalvik.vm.native.bridge). It's not part of the ART APEX and not
   known statically, so the system namespace should be used.

4. libnativeloader also loads JNI libraries from classloader
   namespaces, but has a fallback if no such namespace can be found
   based on caller location. Fall back to the system namespace to
   handle libraries loaded during the preload phase in the zygote.

5. JVMTI agents are loaded by dalvik.system.VMDebug.attachAgent().
   Treat these too as external libraries - they are loaded in a way
   similar to JNI libraries through OpenNativeLibrary in
   libnativeloader, so are covered by #4.

   They are normally loaded by apps with a classloader, but a special
   case is adbconnection which loads libjdwp.so in the ART APEX without
   one and hence falls back to the system namespace. We therefore need
   to create a link for it (https://r.android.com/1690889,
   https://r.android.com/1690795).

All cases #2-#5 are covered by libnativeloader and libnativebridge.
Introduce OpenSystemLibrary, and since libnativeloader depends on
libnativebridge, put it in the latter to be usable from both. It's only
an internal dependency not exposed in the APEX stubs.

(Another possibility could be to put it in the generic toolbox lib
libartbase, but it's split into -d and non-d variants, and we don't
want to split libnative{loader,bridge} that way.)

Since libnativeloader_test no longer needs to mock dlopen we can
simplify it to a more regular test that loads the tested libs
dynamically.

This relands https://r.android.com/1673312 without setting
ANDROID_ADDITIONAL_PUBLIC_LIBRARIES in run-test-jar, because that made
libnativeloader try to preload internal libraries from the system
namespace.

Test: mmm art
Test: atest art/libnativeloader
Test: atest CtsJniTestCases
Test: Cuttlefish app compat test that uses NDK translation
Test: Manual tests with Android Studio TI agents
  (http://g/art-module-team/7Jy3Tg7LCh0)
Test: art/test/testrunner/testrunner.py --target --64 --optimizing
  in chroot on cuttlefish
Bug: 130340935
Change-Id: I7fb32faacc1c214402b58125d8190e97bbbcfad2

8 files changed, 152 insertions, 60 deletions

diff --git a/libnativebridge/Android.bp b/libnativebridge/Android.bp
index cb07d35363..8e87997b00 100644
--- a/libnativebridge/Android.bp
+++ b/libnativebridge/Android.bp
@@ -10,9 +10,6 @@ package {
 cc_defaults {
     name: "libnativebridge-defaults",
     defaults: ["art_defaults"],
-    cppflags: [
-        "-fvisibility=protected",
-    ],
     header_libs: [
         "jni_headers",
         "libnativebridge-headers",
@@ -64,10 +61,10 @@ art_cc_library {
 
     target: {
         android: {
-            version_script: "libnativebridge.map.txt",
-        },
-        linux: {
-            version_script: "libnativebridge.map.txt",
+            header_libs: [
+                "libnativeloader-headers", // For dlext_namespaces.h
+            ],
+            shared_libs: ["libdl_android"],
         },
     },
 
diff --git a/libnativebridge/include/nativebridge/native_bridge.h b/libnativebridge/include/nativebridge/native_bridge.h
index e20b6270a1..2199bab552 100644
--- a/libnativebridge/include/nativebridge/native_bridge.h
+++ b/libnativebridge/include/nativebridge/native_bridge.h
@@ -29,6 +29,11 @@ namespace android {
 extern "C" {
 #endif  // __cplusplus
 
+// Loads a shared library from the system linker namespace, suitable for
+// platform libraries in /system/lib(64). If linker namespaces don't exist (i.e.
+// on host), this simply calls dlopen().
+void* OpenSystemLibrary(const char* path, int flags);
+
 struct NativeBridgeRuntimeCallbacks;
 struct NativeBridgeRuntimeValues;
 
diff --git a/libnativebridge/native_bridge.cc b/libnativebridge/native_bridge.cc
index 46a05a23bf..fb13d62be0 100644
--- a/libnativebridge/native_bridge.cc
+++ b/libnativebridge/native_bridge.cc
@@ -31,6 +31,10 @@
 #include <android-base/macros.h>
 #include <log/log.h>
 
+#ifdef ART_TARGET_ANDROID
+#include "nativeloader/dlext_namespaces.h"
+#endif
+
 namespace android {
 
 #ifdef __APPLE__
@@ -40,6 +44,28 @@ void UNUSED(const T&) {}
 
 extern "C" {
 
+void* OpenSystemLibrary(const char* path, int flags) {
+#ifdef ART_TARGET_ANDROID
+  // The system namespace is called "default" for binaries in /system and
+  // "system" for those in the Runtime APEX. Try "system" first since
+  // "default" always exists.
+  // TODO(b/185587109): Get rid of this error prone logic.
+  android_namespace_t* system_ns = android_get_exported_namespace("system");
+  if (system_ns == nullptr) {
+    system_ns = android_get_exported_namespace("default");
+    LOG_ALWAYS_FATAL_IF(system_ns == nullptr,
+                        "Failed to get system namespace for loading %s", path);
+  }
+  const android_dlextinfo dlextinfo = {
+      .flags = ANDROID_DLEXT_USE_NAMESPACE,
+      .library_namespace = system_ns,
+  };
+  return android_dlopen_ext(path, flags, &dlextinfo);
+#else
+  return dlopen(path, flags);
+#endif
+}
+
 // Environment values required by the apps running with native bridge.
 struct NativeBridgeRuntimeValues {
     const char* os_arch;
@@ -223,8 +249,12 @@ bool LoadNativeBridge(const char* nb_library_filename,
     if (!NativeBridgeNameAcceptable(nb_library_filename)) {
       CloseNativeBridge(true);
     } else {
-      // Try to open the library.
-      void* handle = dlopen(nb_library_filename, RTLD_LAZY);
+      // Try to open the library. We assume this library is provided by the
+      // platform rather than the ART APEX itself, so use the system namespace
+      // to avoid requiring a static linker config link to it from the
+      // com_android_art namespace.
+      void* handle = OpenSystemLibrary(nb_library_filename, RTLD_LAZY);
+
       if (handle != nullptr) {
         callbacks = reinterpret_cast<NativeBridgeCallbacks*>(dlsym(handle,
                                                                    kNativeBridgeInterfaceSymbol));
diff --git a/libnativeloader/Android.bp b/libnativeloader/Android.bp
index 316c087453..2cc6b80787 100644
--- a/libnativeloader/Android.bp
+++ b/libnativeloader/Android.bp
@@ -12,9 +12,6 @@ package {
 cc_defaults {
     name: "libnativeloader-defaults",
     defaults: ["art_defaults"],
-    cppflags: [
-        "-fvisibility=hidden",
-    ],
     header_libs: ["libnativeloader-headers"],
     export_header_lib_headers: ["libnativeloader-headers"],
 }
@@ -121,30 +118,32 @@ cc_library_headers {
 
 art_cc_test {
     name: "libnativeloader_test",
+    defaults: ["art_test_defaults"],
+    host_supported: false,
+
     srcs: [
         "native_loader_test.cpp",
-        "native_loader.cpp",
-        "library_namespaces.cpp",
-        "native_loader_namespace.cpp",
-        "public_libraries.cpp",
     ],
     cflags: ["-DANDROID"],
-    static_libs: [
-        "libbase",
-        "liblog",
-        "libgmock",
-        "PlatformProperties",
+
+    // The test mocks libdl_android and libnativebridge symbols, so export them
+    // to override the ones loaded from their libs.
+    ldflags: [
+        "-Wl,--export-dynamic-symbol=android_*",
+        "-Wl,--export-dynamic-symbol=NativeBridge*",
     ],
+
     header_libs: [
         "libnativebridge-headers",
         "libnativehelper_header_only",
-        "libnativeloader-headers",
     ],
-    // native_loader_test.cpp mocks libdl APIs so system_shared_libs
-    // are used to include C libraries without libdl.
-    system_shared_libs: [
-        "libc",
-        "libm",
+    static_libs: [
+        "libgmock",
+    ],
+    shared_libs: [
+        "libbase",
+        "libnativeloader",
     ],
+
     test_suites: ["device-tests"],
 }
diff --git a/libnativeloader/library_namespaces.cpp b/libnativeloader/library_namespaces.cpp
index 79fee060cf..59369eee43 100644
--- a/libnativeloader/library_namespaces.cpp
+++ b/libnativeloader/library_namespaces.cpp
@@ -122,16 +122,18 @@ void LibraryNamespaces::Initialize() {
     return;
   }
 
-  // android_init_namespaces() expects all the public libraries
-  // to be loaded so that they can be found by soname alone.
+  // Load the preloadable public libraries. Since libnativeloader is in the
+  // com_android_art namespace, use OpenSystemLibrary rather than dlopen to
+  // ensure the libraries are loaded in the system namespace.
   //
   // TODO(dimitry): this is a bit misleading since we do not know
   // if the vendor public library is going to be opened from /vendor/lib
   // we might as well end up loading them from /system/lib or /product/lib
   // For now we rely on CTS test to catch things like this but
   // it should probably be addressed in the future.
-  for (const auto& soname : android::base::Split(preloadable_public_libraries(), ":")) {
-    LOG_ALWAYS_FATAL_IF(dlopen(soname.c_str(), RTLD_NOW | RTLD_NODELETE) == nullptr,
+  for (const std::string& soname : android::base::Split(preloadable_public_libraries(), ":")) {
+    void* handle = OpenSystemLibrary(soname.c_str(), RTLD_NOW | RTLD_NODELETE);
+    LOG_ALWAYS_FATAL_IF(handle == nullptr,
                         "Error preloading public library %s: %s", soname.c_str(), dlerror());
   }
 }
diff --git a/libnativeloader/native_loader.cpp b/libnativeloader/native_loader.cpp
index b34692ae16..30c7b5a377 100644
--- a/libnativeloader/native_loader.cpp
+++ b/libnativeloader/native_loader.cpp
@@ -254,7 +254,11 @@ void* OpenNativeLibrary(JNIEnv* env, int32_t target_sdk_version, const char* pat
       }
     }
 
-    void* handle = dlopen(path, RTLD_NOW);
+    // Fall back to the system namespace. This happens for preloaded JNI
+    // libraries in the zygote.
+    // TODO(b/185833744): Investigate if this should fall back to the app main
+    // namespace (aka anonymous namespace) instead.
+    void* handle = OpenSystemLibrary(path, RTLD_NOW);
     if (handle == nullptr) {
       *error_msg = strdup(dlerror());
     }
diff --git a/libnativeloader/native_loader_test.cpp b/libnativeloader/native_loader_test.cpp
index 43c3c151c9..e754414f46 100644
--- a/libnativeloader/native_loader_test.cpp
+++ b/libnativeloader/native_loader_test.cpp
@@ -36,12 +36,13 @@ namespace android {
 namespace nativeloader {
 
 using ::testing::Eq;
+using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::StrEq;
 using ::testing::_;
 using internal::ConfigEntry;
-using internal::ParseConfig;
 using internal::ParseApexLibrariesConfig;
+using internal::ParseConfig;
 
 #if defined(__LP64__)
 #define LIB_DIR "lib64"
@@ -49,19 +50,15 @@ using internal::ParseApexLibrariesConfig;
 #define LIB_DIR "lib"
 #endif
 
-// gmock interface that represents interested platform APIs on libdl and libnativebridge
+// gmock interface that represents interested platform APIs on libdl_android and libnativebridge
 class Platform {
  public:
   virtual ~Platform() {}
 
-  // libdl APIs
-  virtual void* dlopen(const char* filename, int flags) = 0;
-  virtual int dlclose(void* handle) = 0;
-  virtual char* dlerror(void) = 0;
-
-  // These mock_* are the APIs semantically the same across libdl and libnativebridge.
+  // These mock_* are the APIs semantically the same across libdl_android and libnativebridge.
   // Instead of having two set of mock APIs for the two, define only one set with an additional
-  // argument 'bool bridged' to identify the context (i.e., called for libdl or libnativebridge).
+  // argument 'bool bridged' to identify the context (i.e., called for libdl_android or
+  // libnativebridge).
   typedef char* mock_namespace_handle;
   virtual bool mock_init_anonymous_namespace(bool bridged, const char* sonames,
                                              const char* search_paths) = 0;
@@ -74,7 +71,7 @@ class Platform {
   virtual void* mock_dlopen_ext(bool bridged, const char* filename, int flags,
                                 mock_namespace_handle ns) = 0;
 
-  // libnativebridge APIs for which libdl has no corresponding APIs
+  // libnativebridge APIs for which libdl_android has no corresponding APIs
   virtual bool NativeBridgeInitialized() = 0;
   virtual const char* NativeBridgeGetError() = 0;
   virtual bool NativeBridgeIsPathSupported(const char*) = 0;
@@ -125,11 +122,6 @@ class MockPlatform : public Platform {
         }));
   }
 
-  // Mocking libdl APIs
-  MOCK_METHOD2(dlopen, void*(const char*, int));
-  MOCK_METHOD1(dlclose, int(void*));
-  MOCK_METHOD0(dlerror, char*());
-
   // Mocking the common APIs
   MOCK_METHOD3(mock_init_anonymous_namespace, bool(bool, const char*, const char*));
   MOCK_METHOD7(mock_create_namespace,
@@ -155,19 +147,11 @@ class MockPlatform : public Platform {
 
 static std::unique_ptr<MockPlatform> mock;
 
-// Provide C wrappers for the mock object.
+// Provide C wrappers for the mock object. These symbols must be exported by ld
+// to be able to override the real symbols in the shared libs.
 extern "C" {
-void* dlopen(const char* file, int flag) {
-  return mock->dlopen(file, flag);
-}
 
-int dlclose(void* handle) {
-  return mock->dlclose(handle);
-}
-
-char* dlerror(void) {
-  return mock->dlerror();
-}
+// libdl_android APIs
 
 bool android_init_anonymous_namespace(const char* sonames, const char* search_path) {
   return mock->mock_init_anonymous_namespace(false, sonames, search_path);
@@ -197,6 +181,7 @@ void* android_dlopen_ext(const char* filename, int flags, const android_dlextinf
 }
 
 // libnativebridge APIs
+
 bool NativeBridgeIsSupported(const char* libpath) {
   return mock->NativeBridgeIsSupported(libpath);
 }
@@ -313,7 +298,8 @@ class NativeLoaderTest : public ::testing::TestWithParam<bool> {
     std::vector<std::string> default_public_libs =
         android::base::Split(preloadable_public_libraries(), ":");
     for (auto l : default_public_libs) {
-      EXPECT_CALL(*mock, dlopen(StrEq(l.c_str()), RTLD_NOW | RTLD_NODELETE))
+      EXPECT_CALL(*mock,
+                  mock_dlopen_ext(false, StrEq(l.c_str()), RTLD_NOW | RTLD_NODELETE, NotNull()))
           .WillOnce(Return(any_nonnull));
     }
   }
@@ -336,6 +322,76 @@ TEST_P(NativeLoaderTest, InitializeLoadsDefaultPublicLibraries) {
   RunTest();
 }
 
+TEST_P(NativeLoaderTest, OpenNativeLibraryWithoutClassloaderInApex) {
+  const char* test_lib_path = "libfoo.so";
+  void* fake_handle = &fake_handle;  // Arbitrary non-null value
+  EXPECT_CALL(*mock,
+              mock_dlopen_ext(false, StrEq(test_lib_path), RTLD_NOW, NsEq("com_android_art")))
+      .WillOnce(Return(fake_handle));
+
+  bool needs_native_bridge = false;
+  char* errmsg = nullptr;
+  EXPECT_EQ(fake_handle,
+            OpenNativeLibrary(env.get(),
+                              /*target_sdk_version=*/17,
+                              test_lib_path,
+                              /*class_loader=*/nullptr,
+                              /*caller_location=*/"/apex/com.android.art/javalib/myloadinglib.jar",
+                              /*library_path=*/nullptr,
+                              &needs_native_bridge,
+                              &errmsg));
+  // OpenNativeLibrary never uses nativebridge when there's no classloader. That
+  // should maybe change.
+  EXPECT_EQ(needs_native_bridge, false);
+  EXPECT_EQ(errmsg, nullptr);
+}
+
+TEST_P(NativeLoaderTest, OpenNativeLibraryWithoutClassloaderInFramework) {
+  const char* test_lib_path = "libfoo.so";
+  void* fake_handle = &fake_handle;  // Arbitrary non-null value
+  EXPECT_CALL(*mock, mock_dlopen_ext(false, StrEq(test_lib_path), RTLD_NOW, NsEq("system")))
+      .WillOnce(Return(fake_handle));
+
+  bool needs_native_bridge = false;
+  char* errmsg = nullptr;
+  EXPECT_EQ(fake_handle,
+            OpenNativeLibrary(env.get(),
+                              /*target_sdk_version=*/17,
+                              test_lib_path,
+                              /*class_loader=*/nullptr,
+                              /*caller_location=*/"/system/framework/framework.jar!classes1.dex",
+                              /*library_path=*/nullptr,
+                              &needs_native_bridge,
+                              &errmsg));
+  // OpenNativeLibrary never uses nativebridge when there's no classloader. That
+  // should maybe change.
+  EXPECT_EQ(needs_native_bridge, false);
+  EXPECT_EQ(errmsg, nullptr);
+}
+
+TEST_P(NativeLoaderTest, OpenNativeLibraryWithoutClassloaderAndCallerLocation) {
+  const char* test_lib_path = "libfoo.so";
+  void* fake_handle = &fake_handle;  // Arbitrary non-null value
+  EXPECT_CALL(*mock, mock_dlopen_ext(false, StrEq(test_lib_path), RTLD_NOW, NsEq("system")))
+      .WillOnce(Return(fake_handle));
+
+  bool needs_native_bridge = false;
+  char* errmsg = nullptr;
+  EXPECT_EQ(fake_handle,
+            OpenNativeLibrary(env.get(),
+                              /*target_sdk_version=*/17,
+                              test_lib_path,
+                              /*class_loader=*/nullptr,
+                              /*caller_location=*/nullptr,
+                              /*library_path=*/nullptr,
+                              &needs_native_bridge,
+                              &errmsg));
+  // OpenNativeLibrary never uses nativebridge when there's no classloader. That
+  // should maybe change.
+  EXPECT_EQ(needs_native_bridge, false);
+  EXPECT_EQ(errmsg, nullptr);
+}
+
 INSTANTIATE_TEST_SUITE_P(NativeLoaderTests, NativeLoaderTest, testing::Bool());
 
 /////////////////////////////////////////////////////////////////
diff --git a/test/etc/run-test-jar b/test/etc/run-test-jar
index 86eb722e6b..ba3c4eb60f 100755
--- a/test/etc/run-test-jar
+++ b/test/etc/run-test-jar
@@ -1234,7 +1234,6 @@ if [ "$HOST" = "n" ]; then
     cmdline="cd $DEX_LOCATION && \
              export ASAN_OPTIONS=$RUN_TEST_ASAN_OPTIONS && \
              export ANDROID_DATA=$DEX_LOCATION && \
-             export ANDROID_ADDITIONAL_PUBLIC_LIBRARIES=$PUBLIC_LIBS && \
              export DEX_LOCATION=$DEX_LOCATION && \
              export ANDROID_ROOT=$ANDROID_ROOT && \
              export ANDROID_I18N_ROOT=$ANDROID_I18N_ROOT && \