Revert "Give full access to native libs from java libs in the sa..."

Revert submission 2933611-libnativeloader-shared-syslibs

Reason for revert: Fixing test breakage b/326622518, b/326631342

Reverted changes: /q/submissionid:2933611-libnativeloader-shared-syslibs

Change-Id: I746478191c0e3a2d1a36d87e7a3db980de196420

libnativeloader/Android.bp

@@ -34,7 +34,6 @@ art_cc_library {
         "native_loader.cpp",
     ],
     header_libs: [
-        "art_libartbase_headers",
         "libnativehelper_header_only",
     ],
     shared_libs: [
@@ -67,7 +66,6 @@ art_cc_library {
             ],
             static_libs: [
                 "libPlatformProperties",
-                "libmodules-utils-build",
             ],
         },
     },

libnativeloader/library_namespaces.cpp

@@ -85,7 +85,6 @@ constexpr const char* kProductLibPath = "/product/" LIB ":/system/product/" LIB;
 
 const std::regex kVendorPathRegex("(/system)?/vendor/.*");
 const std::regex kProductPathRegex("(/system)?/product/.*");
-const std::regex kSystemPathRegex("/system(_ext)?/.*");  // MUST be tested last.
 
 jobject GetParentClassLoader(JNIEnv* env, jobject class_loader) {
   jclass class_loader_class = env->FindClass("java/lang/ClassLoader");
@@ -104,9 +103,6 @@ ApiDomain GetApiDomainFromPath(const std::string_view path) {
   if (is_product_treblelized() && std::regex_match(path.begin(), path.end(), kProductPathRegex)) {
     return API_DOMAIN_PRODUCT;
   }
-  if (std::regex_match(path.begin(), path.end(), kSystemPathRegex)) {
-    return API_DOMAIN_SYSTEM;
-  }
   return API_DOMAIN_DEFAULT;
 }
 

libnativeloader/library_namespaces.h

@@ -53,7 +53,6 @@ using ApiDomain = enum {
   API_DOMAIN_DEFAULT = 0,  // Locations other than those below, in particular for ordinary apps
   API_DOMAIN_VENDOR = 1,   // Vendor partition
   API_DOMAIN_PRODUCT = 2,  // Product partition
-  API_DOMAIN_SYSTEM = 3,   // System and system_ext partitions
 };
 
 ApiDomain GetApiDomainFromPath(const std::string_view path);

libnativeloader/library_namespaces_test.cpp

@@ -31,45 +31,40 @@ using ::android::base::testing::HasValue;
 using ::android::base::testing::WithMessage;
 using ::testing::StartsWith;
 
-static ApiDomain GetProductApiDomain(ApiDomain fallback_domain) {
+TEST(LibraryNamespacesTest, TestGetApiDomainFromPath) {
   // GetApiDomainFromPath returns API_DOMAIN_PRODUCT only if the device is
   // trebleized and has an unbundled product partition.
-  return is_product_treblelized() ? API_DOMAIN_PRODUCT : fallback_domain;
-}
+  ApiDomain api_domain_product = is_product_treblelized() ? API_DOMAIN_PRODUCT : API_DOMAIN_DEFAULT;
 
-TEST(LibraryNamespacesTest, TestGetApiDomainFromPath) {
   EXPECT_EQ(GetApiDomainFromPath("/data/somewhere"), API_DOMAIN_DEFAULT);
-  EXPECT_EQ(GetApiDomainFromPath("/system/somewhere"), API_DOMAIN_SYSTEM);
-  EXPECT_EQ(GetApiDomainFromPath("/system_ext/somewhere"), API_DOMAIN_SYSTEM);
-  EXPECT_EQ(GetApiDomainFromPath("/systemext/somewhere"), API_DOMAIN_DEFAULT);
-  EXPECT_EQ(GetApiDomainFromPath("/product/somewhere"), GetProductApiDomain(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"),
-            GetProductApiDomain(API_DOMAIN_SYSTEM));
+  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/"), GetProductApiDomain(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"),
-            GetProductApiDomain(API_DOMAIN_DEFAULT));
+  EXPECT_EQ(GetApiDomainFromPath("/product/somewhere:/vendor/somewhere"), api_domain_product);
 }
 
 TEST(LibraryNamespacesTest, TestGetApiDomainFromPathList) {
+  // GetApiDomainFromPath returns API_DOMAIN_PRODUCT only if the device is
+  // trebleized and has an unbundled product partition.
+  ApiDomain api_domain_product = is_product_treblelized() ? API_DOMAIN_PRODUCT : API_DOMAIN_DEFAULT;
+
   EXPECT_THAT(GetApiDomainFromPathList("/data/somewhere"), HasValue(API_DOMAIN_DEFAULT));
-  EXPECT_THAT(GetApiDomainFromPathList("/system/somewhere"), HasValue(API_DOMAIN_SYSTEM));
-  EXPECT_THAT(GetApiDomainFromPathList("/system_ext/somewhere"), HasValue(API_DOMAIN_SYSTEM));
-  EXPECT_THAT(GetApiDomainFromPathList("/product/somewhere"),
-              HasValue(GetProductApiDomain(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(GetProductApiDomain(API_DOMAIN_SYSTEM)));
+  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));
@@ -78,17 +73,13 @@ TEST(LibraryNamespacesTest, TestGetApiDomainFromPathList) {
   EXPECT_THAT(GetApiDomainFromPathList("/vendor/somewhere:"), HasValue(API_DOMAIN_VENDOR));
 
   EXPECT_THAT(GetApiDomainFromPathList("/data/somewhere:/product/somewhere"),
-              HasValue(GetProductApiDomain(API_DOMAIN_DEFAULT)));
-  if (GetProductApiDomain(API_DOMAIN_DEFAULT) == API_DOMAIN_PRODUCT) {
+              HasValue(api_domain_product));
+  if (api_domain_product == 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"))));
   }
-  EXPECT_THAT(GetApiDomainFromPathList("/system/somewhere:/vendor/somewhere"),
-              HasError(WithMessage(StartsWith("Path list crosses partition boundaries"))));
-  EXPECT_THAT(GetApiDomainFromPathList("/system_ext/somewhere:/vendor/somewhere"),
-              HasError(WithMessage(StartsWith("Path list crosses partition boundaries"))));
 }
 
 }  // namespace

libnativeloader/native_loader.cpp

@@ -25,21 +25,18 @@
 #include <memory>
 #include <mutex>
 #include <optional>
-#include <regex>
 #include <string>
 #include <vector>
 
 #include "android-base/file.h"
 #include "android-base/macros.h"
 #include "android-base/strings.h"
 #include "android-base/thread_annotations.h"
-#include "base/macros.h"
 #include "nativebridge/native_bridge.h"
 #include "nativehelper/scoped_utf_chars.h"
 #include "public_libraries.h"
 
 #ifdef ART_TARGET_ANDROID
-#include "android-modules-utils/sdk_level.h"
 #include "library_namespaces.h"
 #include "log/log.h"
 #include "nativeloader/dlext_namespaces.h"
@@ -54,9 +51,6 @@ namespace {
 using ::android::base::Result;
 using ::android::nativeloader::LibraryNamespaces;
 
-const std::regex kPartitionNativeLibPathRegex(
-    "/(system(_ext)?|(system/)?(vendor|product))/lib(64)?/.*");
-
 // NATIVELOADER_DEFAULT_NAMESPACE_LIBS is an environment variable that can be
 // used to list extra libraries (separated by ":") that libnativeloader will
 // load from the default namespace. The libraries must be listed without paths,
@@ -93,23 +87,6 @@ std::optional<NativeLoaderNamespace> FindApexNamespace(const char* caller_locati
   return std::nullopt;
 }
 
-Result<NativeLoaderNamespace> GetNamespaceForApiDomain(nativeloader::ApiDomain api_domain,
-                                                       bool is_bridged) {
-  switch (api_domain) {
-    case nativeloader::API_DOMAIN_VENDOR:
-      return NativeLoaderNamespace::GetExportedNamespace(nativeloader::kVendorNamespaceName,
-                                                         is_bridged);
-    case nativeloader::API_DOMAIN_PRODUCT:
-      return NativeLoaderNamespace::GetExportedNamespace(nativeloader::kProductNamespaceName,
-                                                         is_bridged);
-    case nativeloader::API_DOMAIN_SYSTEM:
-      return NativeLoaderNamespace::GetSystemNamespace(is_bridged);
-    default:
-      LOG_FATAL("Invalid API domain %d", api_domain);
-      UNREACHABLE();
-  }
-}
-
 Result<void> CreateNativeloaderDefaultNamespaceLibsLink(NativeLoaderNamespace& ns)
     REQUIRES(g_namespaces_mutex) {
   const char* links = getenv("NATIVELOADER_DEFAULT_NAMESPACE_LIBS");
@@ -334,61 +311,6 @@ void* OpenNativeLibrary(JNIEnv* env,
     return handle;
   }
 
-  // If the caller is in any of the system image partitions and the library is
-  // in the same partition then load it without regards to public library
-  // restrictions. This is only done if the library is specified by an absolute
-  // path, so we don't affect the lookup process for libraries specified by name
-  // only.
-  if (caller_location != nullptr &&
-      // Check that the library is in the partition-wide native library
-      // location. Apps in the partition may have their own native libraries,
-      // and those should still be loaded with the app's classloader namespace.
-      std::regex_match(path, kPartitionNativeLibPathRegex) &&
-      // Don't do this if the system image is older than V, to avoid any compat
-      // issues with apps and shared libs in them.
-      android::modules::sdklevel::IsAtLeastV()) {
-    nativeloader::ApiDomain caller_api_domain = nativeloader::GetApiDomainFromPath(caller_location);
-    if (caller_api_domain != nativeloader::API_DOMAIN_DEFAULT) {
-      nativeloader::ApiDomain library_api_domain = nativeloader::GetApiDomainFromPath(path);
-
-      if (library_api_domain == caller_api_domain) {
-        bool is_bridged = false;
-        if (library_path_j != nullptr) {
-          ScopedUtfChars library_path_utf_chars(env, library_path_j);
-          if (library_path_utf_chars[0] != '\0') {
-            is_bridged = NativeBridgeIsPathSupported(library_path_utf_chars.c_str());
-          }
-        }
-
-        Result<NativeLoaderNamespace> ns = GetNamespaceForApiDomain(caller_api_domain, is_bridged);
-        if (!ns.ok()) {
-          ALOGD("Failed to find ns for caller %s in API domain %d to load %s (is_bridged=%b): %s",
-                caller_location,
-                caller_api_domain,
-                path,
-                is_bridged,
-                ns.error().message().c_str());
-          *error_msg = strdup(ns.error().message().c_str());
-          return nullptr;
-        }
-
-        *needs_native_bridge = ns.value().IsBridged();
-        Result<void*> handle = ns.value().Load(path);
-        ALOGD("Load %s using ns %s for caller %s in same partition (is_bridged=%b): %s",
-              path,
-              ns.value().name().c_str(),
-              caller_location,
-              is_bridged,
-              handle.ok() ? "ok" : handle.error().message().c_str());
-        if (!handle.ok()) {
-          *error_msg = strdup(handle.error().message().c_str());
-          return nullptr;
-        }
-        return handle.value();
-      }
-    }
-  }
-
   std::lock_guard<std::mutex> guard(g_namespaces_mutex);
 
   {

libnativeloader/test/src/android/test/app/DataAppTest.java

@@ -67,18 +67,11 @@ public void testLoadExtendedPublicLibrariesViaSystemSharedLib() {
 
     @Test
     public void testLoadPrivateLibrariesViaSystemSharedLib() {
-        if (TestUtils.canLoadPrivateLibsFromSamePartition()) {
-            // TODO(b/186729817): These loads work because the findLibrary call in
-            // loadLibrary0 in Runtime.java searches the system libs and converts
-            // them to absolute paths.
-            SystemSharedLib.loadLibrary("system_private2");
-            SystemSharedLib.loadLibrary("systemext_private2");
-        } else {
-            TestUtils.assertLibraryInaccessible(
-                    () -> { SystemSharedLib.loadLibrary("system_private2"); });
-            TestUtils.assertLibraryInaccessible(
-                    () -> { SystemSharedLib.loadLibrary("systemext_private2"); });
-        }
+        // TODO(b/237577392): Loading a private native system library via a shared system library
+        // ought to work.
+        TestUtils.assertLibraryInaccessible(() -> SystemSharedLib.loadLibrary("system_private2"));
+        TestUtils.assertLibraryInaccessible(
+                () -> SystemSharedLib.loadLibrary("systemext_private2"));
 
         if (!TestUtils.skipPublicProductLibTests()) {
             TestUtils.assertLibraryInaccessible(
@@ -90,18 +83,12 @@ public void testLoadPrivateLibrariesViaSystemSharedLib() {
 
     @Test
     public void testLoadPrivateLibrariesViaSystemExtSharedLib() {
-        if (TestUtils.canLoadPrivateLibsFromSamePartition()) {
-            // TODO(b/186729817): These loads work because the findLibrary call in
-            // loadLibrary0 in Runtime.java searches the system libs and converts
-            // them to absolute paths.
-            SystemExtSharedLib.loadLibrary("system_private3");
-            SystemExtSharedLib.loadLibrary("systemext_private3");
-        } else {
-            TestUtils.assertLibraryInaccessible(
-                    () -> { SystemExtSharedLib.loadLibrary("system_private3"); });
-            TestUtils.assertLibraryInaccessible(
-                    () -> { SystemExtSharedLib.loadLibrary("systemext_private3"); });
-        }
+        // TODO(b/237577392): Loading a private native system library via a shared system library
+        // ought to work.
+        TestUtils.assertLibraryInaccessible(
+                () -> SystemExtSharedLib.loadLibrary("system_private3"));
+        TestUtils.assertLibraryInaccessible(
+                () -> SystemExtSharedLib.loadLibrary("systemext_private3"));
 
         if (!TestUtils.skipPublicProductLibTests()) {
             TestUtils.assertLibraryInaccessible(

libnativeloader/test/src/android/test/app/ProductAppTest.java

@@ -72,21 +72,14 @@ public void testLoadExtendedPublicLibrariesViaSystemSharedLib() {
 
     @Test
     public void testLoadPrivateLibrariesViaSystemSharedLib() {
-        if (TestUtils.productAppsAreShared() || TestUtils.canLoadPrivateLibsFromSamePartition()) {
-            // TODO(b/186729817): These loads work in the
-            // canLoadPrivateLibsFromSamePartition case because the findLibrary
-            // call in loadLibrary0 in Runtime.java searches the system libs and
-            // converts them to absolute paths.
-            SystemSharedLib.loadLibrary("system_private2");
-            SystemSharedLib.loadLibrary("systemext_private2");
-        } else {
+        if (!TestUtils.productAppsAreShared()) {
+            // TODO(b/237577392): Loading a private native system library via a shared system
+            // library ought to work.
             TestUtils.assertLibraryInaccessible(
                     () -> SystemSharedLib.loadLibrary("system_private2"));
             TestUtils.assertLibraryInaccessible(
                     () -> SystemSharedLib.loadLibrary("systemext_private2"));
-        }
 
-        if (!TestUtils.productAppsAreShared()) {
             TestUtils.assertLibraryInaccessible(
                     () -> SystemSharedLib.loadLibrary("product_private2"));
         }
@@ -96,21 +89,14 @@ public void testLoadPrivateLibrariesViaSystemSharedLib() {
 
     @Test
     public void testLoadPrivateLibrariesViaSystemExtSharedLib() {
-        if (TestUtils.productAppsAreShared() || TestUtils.canLoadPrivateLibsFromSamePartition()) {
-            // TODO(b/186729817): These loads work in the
-            // canLoadPrivateLibsFromSamePartition case because the findLibrary
-            // call in loadLibrary0 in Runtime.java searches the system libs and
-            // converts them to absolute paths.
-            SystemExtSharedLib.loadLibrary("system_private3");
-            SystemExtSharedLib.loadLibrary("systemext_private3");
-        } else {
+        if (!TestUtils.productAppsAreShared()) {
+            // TODO(b/237577392): Loading a private native system library via a shared system
+            // library ought to work.
             TestUtils.assertLibraryInaccessible(
                     () -> SystemExtSharedLib.loadLibrary("system_private3"));
             TestUtils.assertLibraryInaccessible(
                     () -> SystemExtSharedLib.loadLibrary("systemext_private3"));
-        }
 
-        if (!TestUtils.productAppsAreShared()) {
             TestUtils.assertLibraryInaccessible(
                     () -> SystemExtSharedLib.loadLibrary("product_private3"));
         }

libnativeloader/test/src/android/test/app/VendorAppTest.java

@@ -71,18 +71,11 @@ public void testLoadExtendedPublicLibrariesViaSystemSharedLib() {
 
     @Test
     public void testLoadPrivateLibrariesViaSystemSharedLib() {
-        if (TestUtils.canLoadPrivateLibsFromSamePartition()) {
-            // TODO(b/186729817): These loads work because the findLibrary call in
-            // loadLibrary0 in Runtime.java searches the system libs and converts
-            // them to absolute paths.
-            SystemSharedLib.loadLibrary("system_private2");
-            SystemSharedLib.loadLibrary("systemext_private2");
-        } else {
-            TestUtils.assertLibraryInaccessible(
-                    () -> SystemSharedLib.loadLibrary("system_private2"));
-            TestUtils.assertLibraryInaccessible(
-                    () -> SystemSharedLib.loadLibrary("systemext_private2"));
-        }
+        // TODO(b/237577392): Loading a private native system library via a shared system library
+        // ought to work.
+        TestUtils.assertLibraryInaccessible(() -> SystemSharedLib.loadLibrary("system_private2"));
+        TestUtils.assertLibraryInaccessible(
+                () -> SystemSharedLib.loadLibrary("systemext_private2"));
 
         if (!TestUtils.skipPublicProductLibTests()) {
             TestUtils.assertLibraryInaccessible(
@@ -94,18 +87,12 @@ public void testLoadPrivateLibrariesViaSystemSharedLib() {
 
     @Test
     public void testLoadPrivateLibrariesViaSystemExtSharedLib() {
-        if (TestUtils.canLoadPrivateLibsFromSamePartition()) {
-            // TODO(b/186729817): These loads work because the findLibrary call in
-            // loadLibrary0 in Runtime.java searches the system libs and converts
-            // them to absolute paths.
-            SystemExtSharedLib.loadLibrary("system_private3");
-            SystemExtSharedLib.loadLibrary("systemext_private3");
-        } else {
-            TestUtils.assertLibraryInaccessible(
-                    () -> SystemExtSharedLib.loadLibrary("system_private3"));
-            TestUtils.assertLibraryInaccessible(
-                    () -> SystemExtSharedLib.loadLibrary("systemext_private3"));
-        }
+        // TODO(b/237577392): Loading a private native system library via a shared system library
+        // ought to work.
+        TestUtils.assertLibraryInaccessible(
+                () -> SystemExtSharedLib.loadLibrary("system_private3"));
+        TestUtils.assertLibraryInaccessible(
+                () -> SystemExtSharedLib.loadLibrary("systemext_private3"));
 
         if (!TestUtils.skipPublicProductLibTests()) {
             TestUtils.assertLibraryInaccessible(