From b37c4818da5f3f40036ae56ccffb670c9ff2cb97 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Thu, 16 May 2019 21:03:30 +0900 Subject: [PATCH] Don't create anonymous namespace Don't create anonymous namespace separately, use the first namespace that is created for app classloader as the anonymous namespace. Note that the anonymous namespace is set via the new ANDROID_NAMESPACE_TYPE_ALSO_USED_AS_ANONYMOUS. I didn't creat a new function like android_set_anonymous_namespace as it requires uprev of the libnativebridge interface and makes it harder to delete the old android_init_anonymous_namespace as we have to keep it until all proprietary bridged loaders are updated. Bug: 130388701 Test: CtsBionicTestCases Test: run games on http://www.monogame.net/showcase/?Android Change-Id: I0fdd614365eaa56c4ab47538bf3772d94bd9ae55 --- .../include/nativeloader/dlext_namespaces.h | 26 +++++---- libnativeloader/library_namespaces.cpp | 53 ++++++------------- libnativeloader/library_namespaces.h | 2 + libnativeloader/native_loader_namespace.cpp | 13 ++++- libnativeloader/native_loader_namespace.h | 3 +- libnativeloader/native_loader_test.cpp | 50 +++++++++-------- 6 files changed, 72 insertions(+), 75 deletions(-) diff --git a/libnativeloader/include/nativeloader/dlext_namespaces.h b/libnativeloader/include/nativeloader/dlext_namespaces.h index 2d6ce8569..8937636cf 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 @@ enum { */ 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 a9eea8cc9..7f5768c42 100644 --- a/libnativeloader/library_namespaces.cpp +++ b/libnativeloader/library_namespaces.cpp @@ -159,13 +159,6 @@ Result LibraryNamespaces::Create(JNIEnv* env, uint32_t t } } - // Initialize the anonymous namespace with the first non-empty library path. - Result 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 @@ Result LibraryNamespaces::Create(JNIEnv* env, uint32_t t // 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 @@ Result LibraryNamespaces::Create(JNIEnv* env, uint32_t t } 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 @@ NativeLoaderNamespace* LibraryNamespaces::FindNamespaceByClassLoader(JNIEnv* env return nullptr; } -Result 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 e54bc0a56..84cabde69 100644 --- a/libnativeloader/library_namespaces.h +++ b/libnativeloader/library_namespaces.h @@ -48,6 +48,7 @@ class LibraryNamespaces { void Reset() { namespaces_.clear(); initialized_ = false; + app_main_namespace_ = nullptr; } Result Create(JNIEnv* env, uint32_t target_sdk_version, jobject class_loader, bool is_shared, jstring dex_path, @@ -59,6 +60,7 @@ class LibraryNamespaces { NativeLoaderNamespace* FindParentNamespaceByClassLoader(JNIEnv* env, jobject class_loader); bool initialized_; + NativeLoaderNamespace* app_main_namespace_; std::list> namespaces_; }; diff --git a/libnativeloader/native_loader_namespace.cpp b/libnativeloader/native_loader_namespace.cpp index 4add6e690..a81fddf0f 100644 --- a/libnativeloader/native_loader_namespace.cpp +++ b/libnativeloader/native_loader_namespace.cpp @@ -85,7 +85,8 @@ Result NativeLoaderNamespace::GetPlatformNamespace(bool i Result 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 @@ Result NativeLoaderNamespace::Create( } 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 29b759cb4..7200ee796 100644 --- a/libnativeloader/native_loader_namespace.h +++ b/libnativeloader/native_loader_namespace.h @@ -39,7 +39,8 @@ struct NativeLoaderNamespace { 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 b939eee25..a641109bc 100644 --- a/libnativeloader/native_loader_test.cpp +++ b/libnativeloader/native_loader_test.cpp @@ -331,7 +331,8 @@ class NativeLoaderTest_Create : public NativeLoaderTest { // 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 @@ class NativeLoaderTest_Create : public NativeLoaderTest { 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 @@ TEST_P(NativeLoaderTest_Create, BundledSystemApp) { 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 @@ TEST_P(NativeLoaderTest_Create, BundledVendorApp) { 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 @@ TEST_P(NativeLoaderTest_Create, BundledProductApp_pre30) { 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 @@ TEST_P(NativeLoaderTest_Create, BundledProductApp_post30) { 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 @@ TEST_P(NativeLoaderTest_Create, UnbundledProductApp_post30) { 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 @@ TEST_P(NativeLoaderTest_Create, TwoApks) { 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 @@ TEST_P(NativeLoaderTest_Create, TwoApks) { // 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 @@ TEST_P(NativeLoaderTest_Create, TwoApks) { INSTANTIATE_TEST_SUITE_P(NativeLoaderTests_Create, NativeLoaderTest_Create, testing::Bool()); -// TODO(b/130388701#comment22) add a test for anonymous namespace - } // namespace nativeloader } // namespace android