Merge "Don't create anonymous namespace"
am: 085a0b4404

Change-Id: I6c6123fc5a90d2d58ec3e8201ac7f7cfc7658d75
diff --git a/libnativeloader/include/nativeloader/dlext_namespaces.h b/libnativeloader/include/nativeloader/dlext_namespaces.h
index 2d6ce85..8937636 100644
--- a/libnativeloader/include/nativeloader/dlext_namespaces.h
+++ b/libnativeloader/include/nativeloader/dlext_namespaces.h
@@ -22,18 +22,6 @@
 
 __BEGIN_DECLS
 
-/*
- * Initializes anonymous namespaces. The shared_libs_sonames is the list of sonames
- * to be shared by default namespace separated by colon. Example: "libc.so:libm.so:libdl.so".
- *
- * The library_search_path is the search path for anonymous namespace. The anonymous namespace
- * is used in the case when linker cannot identify the caller of dlopen/dlsym. This happens
- * for the code not loaded by dynamic linker; for example calls from the mono-compiled code.
- */
-extern bool android_init_anonymous_namespace(const char* shared_libs_sonames,
-                                             const char* library_search_path);
-
-
 enum {
   /* A regular namespace is the namespace with a custom search path that does
    * not impose any restrictions on the location of native libraries.
@@ -62,8 +50,18 @@
    */
   ANDROID_NAMESPACE_TYPE_GREYLIST_ENABLED = 0x08000000,
 
-  ANDROID_NAMESPACE_TYPE_SHARED_ISOLATED = ANDROID_NAMESPACE_TYPE_SHARED |
-                                           ANDROID_NAMESPACE_TYPE_ISOLATED,
+  /* This flag instructs linker to use this namespace as the anonymous
+   * namespace. The anonymous namespace is used in the case when linker cannot
+   * identify the caller of dlopen/dlsym. This happens for the code not loaded
+   * by dynamic linker; for example calls from the mono-compiled code. There can
+   * be only one anonymous namespace in a process. If there already is an
+   * anonymous namespace in the process, using this flag when creating a new
+   * namespace causes an error.
+   */
+  ANDROID_NAMESPACE_TYPE_ALSO_USED_AS_ANONYMOUS = 0x10000000,
+
+  ANDROID_NAMESPACE_TYPE_SHARED_ISOLATED =
+      ANDROID_NAMESPACE_TYPE_SHARED | ANDROID_NAMESPACE_TYPE_ISOLATED,
 };
 
 /*
diff --git a/libnativeloader/library_namespaces.cpp b/libnativeloader/library_namespaces.cpp
index a9eea8c..7f5768c 100644
--- a/libnativeloader/library_namespaces.cpp
+++ b/libnativeloader/library_namespaces.cpp
@@ -159,13 +159,6 @@
     }
   }
 
-  // Initialize the anonymous namespace with the first non-empty library path.
-  Result<void> ret;
-  if (!library_path.empty() && !initialized_ &&
-      !(ret = InitPublicNamespace(library_path.c_str()))) {
-    return ret.error();
-  }
-
   LOG_ALWAYS_FATAL_IF(FindNamespaceByClassLoader(env, class_loader) != nullptr,
                       "There is already a namespace associated with this classloader");
 
@@ -215,13 +208,22 @@
 
   // Create the app namespace
   NativeLoaderNamespace* parent_ns = FindParentNamespaceByClassLoader(env, class_loader);
-  auto app_ns =
-      NativeLoaderNamespace::Create(namespace_name, library_path, permitted_path, parent_ns,
-                                    is_shared, target_sdk_version < 24 /* is_greylist_enabled */);
+  // Heuristic: the first classloader with non-empty library_path is assumed to
+  // be the main classloader for app
+  // TODO(b/139178525) remove this heuristic by determining this in LoadedApk (or its
+  // friends) and then passing it down to here.
+  bool is_main_classloader = app_main_namespace_ == nullptr && !library_path.empty();
+  // Policy: the namespace for the main classloader is also used as the
+  // anonymous namespace.
+  bool also_used_as_anonymous = is_main_classloader;
+  // Note: this function is executed with g_namespaces_mutex held, thus no
+  // racing here.
+  auto app_ns = NativeLoaderNamespace::Create(
+      namespace_name, library_path, permitted_path, parent_ns, is_shared,
+      target_sdk_version < 24 /* is_greylist_enabled */, also_used_as_anonymous);
   if (!app_ns) {
     return app_ns.error();
   }
-
   // ... and link to other namespaces to allow access to some public libraries
   bool is_bridged = app_ns->IsBridged();
 
@@ -278,6 +280,9 @@
   }
 
   namespaces_.push_back(std::make_pair(env->NewWeakGlobalRef(class_loader), *app_ns));
+  if (is_main_classloader) {
+    app_main_namespace_ = &(*app_ns);
+  }
 
   return &(namespaces_.back().second);
 }
@@ -295,32 +300,6 @@
   return nullptr;
 }
 
-Result<void> LibraryNamespaces::InitPublicNamespace(const char* library_path) {
-  // Ask native bride if this apps library path should be handled by it
-  bool is_native_bridge = NativeBridgeIsPathSupported(library_path);
-
-  // (http://b/25844435) - Some apps call dlopen from generated code (mono jited
-  // code is one example) unknown to linker in which  case linker uses anonymous
-  // namespace. The second argument specifies the search path for the anonymous
-  // namespace which is the library_path of the classloader.
-  initialized_ = android_init_anonymous_namespace(default_public_libraries().c_str(),
-                                                  is_native_bridge ? nullptr : library_path);
-  if (!initialized_) {
-    return Error() << dlerror();
-  }
-
-  // and now initialize native bridge namespaces if necessary.
-  if (NativeBridgeInitialized()) {
-    initialized_ = NativeBridgeInitAnonymousNamespace(default_public_libraries().c_str(),
-                                                      is_native_bridge ? library_path : nullptr);
-    if (!initialized_) {
-      return Error() << NativeBridgeGetError();
-    }
-  }
-
-  return {};
-}
-
 NativeLoaderNamespace* LibraryNamespaces::FindParentNamespaceByClassLoader(JNIEnv* env,
                                                                            jobject class_loader) {
   jobject parent_class_loader = GetParentClassLoader(env, class_loader);
diff --git a/libnativeloader/library_namespaces.h b/libnativeloader/library_namespaces.h
index e54bc0a..84cabde 100644
--- a/libnativeloader/library_namespaces.h
+++ b/libnativeloader/library_namespaces.h
@@ -48,6 +48,7 @@
   void Reset() {
     namespaces_.clear();
     initialized_ = false;
+    app_main_namespace_ = nullptr;
   }
   Result<NativeLoaderNamespace*> Create(JNIEnv* env, uint32_t target_sdk_version,
                                         jobject class_loader, bool is_shared, jstring dex_path,
@@ -59,6 +60,7 @@
   NativeLoaderNamespace* FindParentNamespaceByClassLoader(JNIEnv* env, jobject class_loader);
 
   bool initialized_;
+  NativeLoaderNamespace* app_main_namespace_;
   std::list<std::pair<jweak, NativeLoaderNamespace>> namespaces_;
 };
 
diff --git a/libnativeloader/native_loader_namespace.cpp b/libnativeloader/native_loader_namespace.cpp
index 4add6e6..a81fddf 100644
--- a/libnativeloader/native_loader_namespace.cpp
+++ b/libnativeloader/native_loader_namespace.cpp
@@ -85,7 +85,8 @@
 
 Result<NativeLoaderNamespace> NativeLoaderNamespace::Create(
     const std::string& name, const std::string& search_paths, const std::string& permitted_paths,
-    const NativeLoaderNamespace* parent, bool is_shared, bool is_greylist_enabled) {
+    const NativeLoaderNamespace* parent, bool is_shared, bool is_greylist_enabled,
+    bool also_used_as_anonymous) {
   bool is_bridged = false;
   if (parent != nullptr) {
     is_bridged = parent->IsBridged();
@@ -100,7 +101,17 @@
   }
   const NativeLoaderNamespace& effective_parent = parent != nullptr ? *parent : *platform_ns;
 
+  // All namespaces for apps are isolated
   uint64_t type = ANDROID_NAMESPACE_TYPE_ISOLATED;
+
+  // The namespace is also used as the anonymous namespace
+  // which is used when the linker fails to determine the caller address
+  if (also_used_as_anonymous) {
+    type |= ANDROID_NAMESPACE_TYPE_ALSO_USED_AS_ANONYMOUS;
+  }
+
+  // Bundled apps have access to all system libraries that are currently loaded
+  // in the default namespace
   if (is_shared) {
     type |= ANDROID_NAMESPACE_TYPE_SHARED;
   }
diff --git a/libnativeloader/native_loader_namespace.h b/libnativeloader/native_loader_namespace.h
index 29b759c..7200ee7 100644
--- a/libnativeloader/native_loader_namespace.h
+++ b/libnativeloader/native_loader_namespace.h
@@ -39,7 +39,8 @@
                                               const std::string& search_paths,
                                               const std::string& permitted_paths,
                                               const NativeLoaderNamespace* parent, bool is_shared,
-                                              bool is_greylist_enabled);
+                                              bool is_greylist_enabled,
+                                              bool also_used_as_anonymous);
 
   NativeLoaderNamespace(NativeLoaderNamespace&&) = default;
   NativeLoaderNamespace(const NativeLoaderNamespace&) = default;
diff --git a/libnativeloader/native_loader_test.cpp b/libnativeloader/native_loader_test.cpp
index b939eee..a641109 100644
--- a/libnativeloader/native_loader_test.cpp
+++ b/libnativeloader/native_loader_test.cpp
@@ -331,7 +331,8 @@
 
   // expected output (.. for the default test inputs)
   std::string expected_namespace_name = "classloader-namespace";
-  uint64_t expected_namespace_flags = ANDROID_NAMESPACE_TYPE_ISOLATED;
+  uint64_t expected_namespace_flags =
+      ANDROID_NAMESPACE_TYPE_ISOLATED | ANDROID_NAMESPACE_TYPE_ALSO_USED_AS_ANONYMOUS;
   std::string expected_library_path = library_path;
   std::string expected_permitted_path = std::string("/data:/mnt/expand:") + permitted_path;
   std::string expected_parent_namespace = "platform";
@@ -356,17 +357,6 @@
     EXPECT_CALL(*mock, NativeBridgeIsPathSupported(_)).Times(AnyNumber());
     EXPECT_CALL(*mock, NativeBridgeInitialized()).Times(AnyNumber());
 
-    if (IsBridged()) {
-      EXPECT_CALL(*mock,
-                  mock_init_anonymous_namespace(false, StrEq(default_public_libraries()), nullptr))
-          .WillOnce(Return(true));
-
-      EXPECT_CALL(*mock, NativeBridgeInitialized()).WillOnce(Return(true));
-    }
-
-    EXPECT_CALL(*mock, mock_init_anonymous_namespace(
-                           Eq(IsBridged()), StrEq(default_public_libraries()), StrEq(library_path)))
-        .WillOnce(Return(true));
     EXPECT_CALL(*mock, mock_create_namespace(
                            Eq(IsBridged()), StrEq(expected_namespace_name), nullptr,
                            StrEq(expected_library_path), expected_namespace_flags,
@@ -443,7 +433,7 @@
   dex_path = "/system/app/foo/foo.apk";
   is_shared = true;
 
-  expected_namespace_flags = ANDROID_NAMESPACE_TYPE_ISOLATED | ANDROID_NAMESPACE_TYPE_SHARED;
+  expected_namespace_flags |= ANDROID_NAMESPACE_TYPE_SHARED;
   SetExpectations();
   RunTest();
 }
@@ -452,7 +442,7 @@
   dex_path = "/vendor/app/foo/foo.apk";
   is_shared = true;
 
-  expected_namespace_flags = ANDROID_NAMESPACE_TYPE_ISOLATED | ANDROID_NAMESPACE_TYPE_SHARED;
+  expected_namespace_flags |= ANDROID_NAMESPACE_TYPE_SHARED;
   SetExpectations();
   RunTest();
 }
@@ -475,7 +465,7 @@
   dex_path = "/product/app/foo/foo.apk";
   is_shared = true;
 
-  expected_namespace_flags = ANDROID_NAMESPACE_TYPE_ISOLATED | ANDROID_NAMESPACE_TYPE_SHARED;
+  expected_namespace_flags |= ANDROID_NAMESPACE_TYPE_SHARED;
   SetExpectations();
   RunTest();
 }
@@ -485,7 +475,7 @@
   is_shared = true;
   target_sdk_version = 30;
 
-  expected_namespace_flags = ANDROID_NAMESPACE_TYPE_ISOLATED | ANDROID_NAMESPACE_TYPE_SHARED;
+  expected_namespace_flags |= ANDROID_NAMESPACE_TYPE_SHARED;
   SetExpectations();
   RunTest();
 }
@@ -512,6 +502,22 @@
   RunTest();
 }
 
+TEST_P(NativeLoaderTest_Create, NamespaceForSharedLibIsNotUsedAsAnonymousNamespace) {
+  if (IsBridged()) {
+    // There is no shared lib in translated arch
+    // TODO(jiyong): revisit this
+    return;
+  }
+  // compared to apks, for java shared libs, library_path is empty; java shared
+  // libs don't have their own native libs. They use platform's.
+  library_path = "";
+  expected_library_path = library_path;
+  // no ALSO_USED_AS_ANONYMOUS
+  expected_namespace_flags = ANDROID_NAMESPACE_TYPE_ISOLATED;
+  SetExpectations();
+  RunTest();
+}
+
 TEST_P(NativeLoaderTest_Create, TwoApks) {
   SetExpectations();
   const uint32_t second_app_target_sdk_version = 29;
@@ -523,6 +529,8 @@
   const std::string expected_second_app_permitted_path =
       std::string("/data:/mnt/expand:") + second_app_permitted_path;
   const std::string expected_second_app_parent_namespace = "classloader-namespace";
+  // no ALSO_USED_AS_ANONYMOUS
+  const uint64_t expected_second_namespace_flags = ANDROID_NAMESPACE_TYPE_ISOLATED;
 
   // The scenario is that second app is loaded by the first app.
   // So the first app's classloader (`classloader`) is parent of the second
@@ -532,10 +540,10 @@
 
   // namespace for the second app is created. Its parent is set to the namespace
   // of the first app.
-  EXPECT_CALL(*mock, mock_create_namespace(Eq(IsBridged()), StrEq(expected_namespace_name), nullptr,
-                                           StrEq(second_app_library_path), expected_namespace_flags,
-                                           StrEq(expected_second_app_permitted_path),
-                                           NsEq(dex_path.c_str())))
+  EXPECT_CALL(*mock, mock_create_namespace(
+                         Eq(IsBridged()), StrEq(expected_namespace_name), nullptr,
+                         StrEq(second_app_library_path), expected_second_namespace_flags,
+                         StrEq(expected_second_app_permitted_path), NsEq(dex_path.c_str())))
       .WillOnce(Return(TO_MOCK_NAMESPACE(TO_ANDROID_NAMESPACE(second_app_dex_path.c_str()))));
   EXPECT_CALL(*mock, mock_link_namespaces(Eq(IsBridged()), NsEq(second_app_dex_path.c_str()), _, _))
       .WillRepeatedly(Return(true));
@@ -568,7 +576,5 @@
 
 INSTANTIATE_TEST_SUITE_P(NativeLoaderTests_Create, NativeLoaderTest_Create, testing::Bool());
 
-// TODO(b/130388701#comment22) add a test for anonymous namespace
-
 }  // namespace nativeloader
 }  // namespace android