Separate handling of a single path and a list of paths when determining
API domains.
Becomes necessary in a later CL where it gets used on more or less
arbitrary paths where we shouldn't treat colons specially.
The old code crashed with a fatal if a list of paths had both vendor
and product directories in it. This changes that to a more appropriate
error that gets propagated to the java level and becomes an exception
where the classloader is created.
Test: atest libnativeloader_test
Bug: 237577392
Change-Id: I783af87a03de18c65fadcd1fd5a71423ec0c786b
diff --git a/libnativeloader/Android.bp b/libnativeloader/Android.bp
index 16449ac..e9c26c5 100644
--- a/libnativeloader/Android.bp
+++ b/libnativeloader/Android.bp
@@ -158,6 +158,7 @@
"native_loader_test.cpp",
],
srcs: [
+ "library_namespaces_test.cpp",
"native_loader_api_test.c",
"native_loader_test.cpp",
"open_system_library.cpp",
diff --git a/libnativeloader/library_namespaces.cpp b/libnativeloader/library_namespaces.cpp
index 2aeaf88..e2b2729 100644
--- a/libnativeloader/library_namespaces.cpp
+++ b/libnativeloader/library_namespaces.cpp
@@ -22,10 +22,13 @@
#include <dirent.h>
#include <dlfcn.h>
+#include <stdio.h>
+#include <algorithm>
#include <optional>
#include <regex>
#include <string>
+#include <string_view>
#include <vector>
#include "android-base/file.h"
@@ -43,6 +46,8 @@
namespace {
+using ::android::base::Error;
+
constexpr const char* kApexPath = "/apex/";
// clns-XX is a linker namespace that is created for normal apps installed in
@@ -78,8 +83,8 @@
// a symlink to the other.
constexpr const char* kProductLibPath = "/product/" LIB ":/system/product/" LIB;
-const std::regex kVendorPathRegex("(^|:)(/system)?/vendor/");
-const std::regex kProductPathRegex("(^|:)(/system)?/product/");
+const std::regex kVendorPathRegex("(/system)?/vendor/.*");
+const std::regex kProductPathRegex("(/system)?/product/.*");
jobject GetParentClassLoader(JNIEnv* env, jobject class_loader) {
jclass class_loader_class = env->FindClass("java/lang/ClassLoader");
@@ -91,18 +96,41 @@
} // namespace
-ApiDomain GetApiDomainFromPath(const std::string& path) {
- ApiDomain api_domain = API_DOMAIN_DEFAULT;
- if (std::regex_search(path, kVendorPathRegex)) {
- api_domain = API_DOMAIN_VENDOR;
+ApiDomain GetApiDomainFromPath(const std::string_view path) {
+ if (std::regex_match(path.begin(), path.end(), kVendorPathRegex)) {
+ return API_DOMAIN_VENDOR;
}
- if (is_product_treblelized() && std::regex_search(path, kProductPathRegex)) {
- LOG_ALWAYS_FATAL_IF(api_domain == API_DOMAIN_VENDOR,
- "Path matches both vendor and product partitions: %s",
- path.c_str());
- api_domain = API_DOMAIN_PRODUCT;
+ if (is_product_treblelized() && std::regex_match(path.begin(), path.end(), kProductPathRegex)) {
+ return API_DOMAIN_PRODUCT;
}
- return api_domain;
+ return API_DOMAIN_DEFAULT;
+}
+
+// Returns the API domain for a ':'-separated list of paths, or an error if they
+// match more than one.
+Result<ApiDomain> GetApiDomainFromPathList(const std::string& path_list) {
+ ApiDomain result = API_DOMAIN_DEFAULT;
+ size_t start_pos = 0;
+ while (true) {
+ size_t end_pos = path_list.find(':', start_pos);
+ ApiDomain api_domain =
+ GetApiDomainFromPath(std::string_view(path_list).substr(start_pos, end_pos));
+ // Allow mixing API_DOMAIN_DEFAULT with any other domain. That's a bit lax,
+ // since the default e.g. includes /data, which strictly speaking is a
+ // separate domain. However, we keep it this way to not risk compat issues
+ // until we actually need all domains.
+ if (api_domain != API_DOMAIN_DEFAULT) {
+ if (result != API_DOMAIN_DEFAULT && result != api_domain) {
+ return Error() << "Path list crosses partition boundaries: " << path_list;
+ }
+ result = api_domain;
+ }
+ if (end_pos == std::string::npos) {
+ break;
+ }
+ start_pos = end_pos + 1;
+ }
+ return result;
}
void LibraryNamespaces::Initialize() {
diff --git a/libnativeloader/library_namespaces.h b/libnativeloader/library_namespaces.h
index 741d411..ae1cd88 100644
--- a/libnativeloader/library_namespaces.h
+++ b/libnativeloader/library_namespaces.h
@@ -24,6 +24,7 @@
#include <list>
#include <optional>
#include <string>
+#include <string_view>
#include "android-base/result.h"
#include "jni.h"
@@ -54,7 +55,8 @@
API_DOMAIN_PRODUCT = 2, // Product partition
};
-nativeloader::ApiDomain GetApiDomainFromPath(const std::string& path);
+ApiDomain GetApiDomainFromPath(const std::string_view path);
+Result<ApiDomain> GetApiDomainFromPathList(const std::string& path_list);
// LibraryNamespaces is a singleton object that manages NativeLoaderNamespace
// objects for an app process. Its main job is to create (and configure) a new
diff --git a/libnativeloader/library_namespaces_test.cpp b/libnativeloader/library_namespaces_test.cpp
new file mode 100644
index 0000000..2e6647d
--- /dev/null
+++ b/libnativeloader/library_namespaces_test.cpp
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#if defined(ART_TARGET_ANDROID)
+
+#include "library_namespaces.h"
+
+#include "android-base/result-gmock.h"
+#include "gtest/gtest.h"
+
+namespace android {
+namespace nativeloader {
+namespace {
+
+using ::android::base::testing::HasError;
+using ::android::base::testing::HasValue;
+using ::android::base::testing::WithMessage;
+using ::testing::StartsWith;
+
+TEST(LibraryNamespacesTest, TestGetApiDomainFromPath) {
+ EXPECT_EQ(GetApiDomainFromPath("/data/somewhere"), API_DOMAIN_DEFAULT);
+ EXPECT_EQ(GetApiDomainFromPath("/system/somewhere"), API_DOMAIN_DEFAULT);
+ EXPECT_EQ(GetApiDomainFromPath("/product/somewhere"), API_DOMAIN_PRODUCT);
+ EXPECT_EQ(GetApiDomainFromPath("/vendor/somewhere"), API_DOMAIN_VENDOR);
+ EXPECT_EQ(GetApiDomainFromPath("/system/product/somewhere"), API_DOMAIN_PRODUCT);
+ EXPECT_EQ(GetApiDomainFromPath("/system/vendor/somewhere"), API_DOMAIN_VENDOR);
+
+ EXPECT_EQ(GetApiDomainFromPath(""), API_DOMAIN_DEFAULT);
+ EXPECT_EQ(GetApiDomainFromPath("/"), API_DOMAIN_DEFAULT);
+ EXPECT_EQ(GetApiDomainFromPath("product/somewhere"), API_DOMAIN_DEFAULT);
+ EXPECT_EQ(GetApiDomainFromPath("/product"), API_DOMAIN_DEFAULT);
+ EXPECT_EQ(GetApiDomainFromPath("/product/"), API_DOMAIN_PRODUCT);
+ EXPECT_EQ(GetApiDomainFromPath(":/product/"), API_DOMAIN_DEFAULT);
+
+ EXPECT_EQ(GetApiDomainFromPath("/data/somewhere:/product/somewhere"), API_DOMAIN_DEFAULT);
+ EXPECT_EQ(GetApiDomainFromPath("/vendor/somewhere:/product/somewhere"), API_DOMAIN_VENDOR);
+ EXPECT_EQ(GetApiDomainFromPath("/product/somewhere:/vendor/somewhere"), API_DOMAIN_PRODUCT);
+}
+
+TEST(LibraryNamespacesTest, TestGetApiDomainFromPathList) {
+ EXPECT_THAT(GetApiDomainFromPathList("/data/somewhere"), HasValue(API_DOMAIN_DEFAULT));
+ EXPECT_THAT(GetApiDomainFromPathList("/system/somewhere"), HasValue(API_DOMAIN_DEFAULT));
+ EXPECT_THAT(GetApiDomainFromPathList("/product/somewhere"), HasValue(API_DOMAIN_PRODUCT));
+ EXPECT_THAT(GetApiDomainFromPathList("/vendor/somewhere"), HasValue(API_DOMAIN_VENDOR));
+ EXPECT_THAT(GetApiDomainFromPathList("/system/product/somewhere"), HasValue(API_DOMAIN_PRODUCT));
+ EXPECT_THAT(GetApiDomainFromPathList("/system/vendor/somewhere"), HasValue(API_DOMAIN_VENDOR));
+
+ EXPECT_THAT(GetApiDomainFromPathList(""), HasValue(API_DOMAIN_DEFAULT));
+ EXPECT_THAT(GetApiDomainFromPathList(":"), HasValue(API_DOMAIN_DEFAULT));
+ EXPECT_THAT(GetApiDomainFromPathList(":/vendor/somewhere"), HasValue(API_DOMAIN_VENDOR));
+ EXPECT_THAT(GetApiDomainFromPathList("/vendor/somewhere:"), HasValue(API_DOMAIN_VENDOR));
+
+ EXPECT_THAT(GetApiDomainFromPathList("/data/somewhere:/product/somewhere"),
+ HasValue(API_DOMAIN_PRODUCT));
+ EXPECT_THAT(GetApiDomainFromPathList("/vendor/somewhere:/product/somewhere"),
+ HasError(WithMessage(StartsWith("Path list crosses partition boundaries"))));
+ EXPECT_THAT(GetApiDomainFromPathList("/product/somewhere:/vendor/somewhere"),
+ HasError(WithMessage(StartsWith("Path list crosses partition boundaries"))));
+}
+
+} // namespace
+} // namespace nativeloader
+} // namespace android
+
+#endif // ART_TARGET_ANDROID
diff --git a/libnativeloader/native_loader.cpp b/libnativeloader/native_loader.cpp
index 80af4da..6192543 100644
--- a/libnativeloader/native_loader.cpp
+++ b/libnativeloader/native_loader.cpp
@@ -199,6 +199,8 @@
#endif
}
+// dex_path_j may be a ':'-separated list of paths, e.g. when creating a shared
+// library loader - cf. mCodePaths in android.content.pm.SharedLibraryInfo.
jstring CreateClassLoaderNamespace(JNIEnv* env,
int32_t target_sdk_version,
jobject class_loader,
@@ -213,13 +215,17 @@
ScopedUtfChars dex_path_chars(env, dex_path_j);
dex_path = dex_path_chars.c_str();
}
- nativeloader::ApiDomain api_domain = nativeloader::GetApiDomainFromPath(dex_path);
+
+ Result<nativeloader::ApiDomain> api_domain = nativeloader::GetApiDomainFromPathList(dex_path);
+ if (!api_domain.ok()) {
+ return env->NewStringUTF(api_domain.error().message().c_str());
+ }
std::lock_guard<std::mutex> guard(g_namespaces_mutex);
Result<NativeLoaderNamespace*> ns = CreateClassLoaderNamespaceLocked(env,
target_sdk_version,
class_loader,
- api_domain,
+ api_domain.value(),
is_shared,
dex_path,
library_path_j,