Revert "Revert "Allow hidden API access from system libraries""
Libraries like RemoteDisplay provide an APK that an app loads into
its process and which accesses internal APIs on the app's behalf,
without exposing the internals to the app. These libraries are
considered part of the platform, but were not exempt from hidden API
checks because they are not loaded with the boot strap class loader.
This patch adds a new flag to DexFile class which the constructor
sets to true of the canonical location of the newly loaded dex file
starts with "${ANDROID_ROOT}/framework/". Hidden API enforcement
then checks this flag when determining whether the caller of
a hidden class member is allowed to access it or not.
This reverts commit 0127b71a2588efcd1a53c192c5c267157878b010.
Previous CL saw two issues:
- buildbots would set non-existent ANDROID_ROOT for host-side builds
- calling realpath on unquickened dex files would overflow the stack
Bug: 64382372
Bug: 76138670
Bug: 76165623
Bug: 76112393
Bug: 76452688
Bug: 76429651
Test: art/test.py --target -r -b -t 674-hiddenapi
Test: SystemUI APCT test
Change-Id: Ie07a088509002593353965d3d24bf7362b643f40
diff --git a/libdexfile/dex/dex_file.cc b/libdexfile/dex/dex_file.cc
index 4613b40..1f47110 100644
--- a/libdexfile/dex/dex_file.cc
+++ b/libdexfile/dex/dex_file.cc
@@ -121,7 +121,8 @@
num_call_site_ids_(0),
oat_dex_file_(oat_dex_file),
container_(std::move(container)),
- is_compact_dex_(is_compact_dex) {
+ is_compact_dex_(is_compact_dex),
+ is_platform_dex_(false) {
CHECK(begin_ != nullptr) << GetLocation();
CHECK_GT(size_, 0U) << GetLocation();
// Check base (=header) alignment.
diff --git a/libdexfile/dex/dex_file.h b/libdexfile/dex/dex_file.h
index aeb49d2..683a824 100644
--- a/libdexfile/dex/dex_file.h
+++ b/libdexfile/dex/dex_file.h
@@ -992,6 +992,14 @@
ALWAYS_INLINE const StandardDexFile* AsStandardDexFile() const;
ALWAYS_INLINE const CompactDexFile* AsCompactDexFile() const;
+ ALWAYS_INLINE bool IsPlatformDexFile() const {
+ return is_platform_dex_;
+ }
+
+ ALWAYS_INLINE void SetIsPlatformDexFile() {
+ is_platform_dex_ = true;
+ }
+
bool IsInMainSection(const void* addr) const {
return Begin() <= addr && addr < Begin() + Size();
}
@@ -1094,6 +1102,9 @@
// If the dex file is a compact dex file. If false then the dex file is a standard dex file.
const bool is_compact_dex_;
+ // If the dex file is located in /system/framework/.
+ bool is_platform_dex_;
+
friend class DexFileLoader;
friend class DexFileVerifierTest;
friend class OatWriter;
diff --git a/openjdkjvmti/fixed_up_dex_file.cc b/openjdkjvmti/fixed_up_dex_file.cc
index ee83c4a..fcbafe7 100644
--- a/openjdkjvmti/fixed_up_dex_file.cc
+++ b/openjdkjvmti/fixed_up_dex_file.cc
@@ -31,7 +31,6 @@
#include "base/leb128.h"
#include "fixed_up_dex_file.h"
-#include "dex/art_dex_file_loader.h"
#include "dex/dex_file-inl.h"
#include "dex/dex_file_loader.h"
#include "dex/dex_file_verifier.h"
@@ -108,7 +107,12 @@
std::vector<unsigned char> data;
std::unique_ptr<const art::DexFile> new_dex_file;
std::string error;
- const art::ArtDexFileLoader dex_file_loader;
+
+ // Do not use ArtDexFileLoader here. This code runs in a signal handler and
+ // its stack is too small to invoke the required LocationIsOnSystemFramework
+ // (b/76429651). Instead, we use DexFileLoader and copy the IsPlatformDexFile
+ // property from `original` to `new_dex_file`.
+ const art::DexFileLoader dex_file_loader;
if (original.IsCompactDexFile()) {
// Since we are supposed to return a standard dex, convert back using dexlayout. It's OK to do
@@ -159,6 +163,10 @@
return nullptr;
}
+ if (original.IsPlatformDexFile()) {
+ const_cast<art::DexFile*>(new_dex_file.get())->SetIsPlatformDexFile();
+ }
+
DoDexUnquicken(*new_dex_file, original);
RecomputeDexChecksum(const_cast<art::DexFile*>(new_dex_file.get()));
diff --git a/runtime/base/file_utils.cc b/runtime/base/file_utils.cc
index 1cb3b9c..2b3e360 100644
--- a/runtime/base/file_utils.cc
+++ b/runtime/base/file_utils.cc
@@ -319,4 +319,21 @@
return path != nullptr && android::base::StartsWith(path.get(), GetAndroidRoot().c_str());
}
+bool LocationIsOnSystemFramework(const char* location) {
+ std::string error_msg;
+ std::string root_path = GetAndroidRootSafe(&error_msg);
+ if (root_path.empty()) {
+ // Could not find Android root.
+ // TODO(dbrazdil): change to stricter GetAndroidRoot() once b/76452688 is resolved.
+ return false;
+ }
+ std::string framework_path = root_path + "/framework/";
+
+ // Warning: Bionic implementation of realpath() allocates > 12KB on the stack.
+ // Do not run this code on a small stack, e.g. in signal handler.
+ UniqueCPtr<const char[]> path(realpath(location, nullptr));
+ return path != nullptr &&
+ android::base::StartsWith(path.get(), framework_path.c_str());
+}
+
} // namespace art
diff --git a/runtime/base/file_utils.h b/runtime/base/file_utils.h
index 7f691d5..8adb4f7 100644
--- a/runtime/base/file_utils.h
+++ b/runtime/base/file_utils.h
@@ -75,6 +75,9 @@
// Return whether the location is on system (i.e. android root).
bool LocationIsOnSystem(const char* location);
+// Return whether the location is on system/framework (i.e. android_root/framework).
+bool LocationIsOnSystemFramework(const char* location);
+
} // namespace art
#endif // ART_RUNTIME_BASE_FILE_UTILS_H_
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 8b64b8d..2110d04 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -7935,7 +7935,8 @@
}
DCHECK(resolved == nullptr || resolved->GetDeclaringClassUnchecked() != nullptr);
if (resolved != nullptr &&
- hiddenapi::ShouldBlockAccessToMember(resolved, class_loader, hiddenapi::kLinking)) {
+ hiddenapi::ShouldBlockAccessToMember(
+ resolved, class_loader, dex_cache, hiddenapi::kLinking)) {
resolved = nullptr;
}
if (resolved != nullptr) {
@@ -8077,7 +8078,8 @@
resolved = klass->FindClassMethod(dex_cache.Get(), method_idx, image_pointer_size_);
}
if (resolved != nullptr &&
- hiddenapi::ShouldBlockAccessToMember(resolved, class_loader.Get(), hiddenapi::kLinking)) {
+ hiddenapi::ShouldBlockAccessToMember(
+ resolved, class_loader.Get(), dex_cache.Get(), hiddenapi::kLinking)) {
resolved = nullptr;
}
return resolved;
@@ -8156,7 +8158,8 @@
}
if (resolved == nullptr ||
- hiddenapi::ShouldBlockAccessToMember(resolved, class_loader.Get(), hiddenapi::kLinking)) {
+ hiddenapi::ShouldBlockAccessToMember(
+ resolved, class_loader.Get(), dex_cache.Get(), hiddenapi::kLinking)) {
const char* name = dex_file.GetFieldName(field_id);
const char* type = dex_file.GetFieldTypeDescriptor(field_id);
ThrowNoSuchFieldError(is_static ? "static " : "instance ", klass, type, name);
@@ -8189,7 +8192,8 @@
StringPiece type(dex_file.GetFieldTypeDescriptor(field_id));
resolved = mirror::Class::FindField(self, klass, name, type);
if (resolved != nullptr &&
- hiddenapi::ShouldBlockAccessToMember(resolved, class_loader.Get(), hiddenapi::kLinking)) {
+ hiddenapi::ShouldBlockAccessToMember(
+ resolved, class_loader.Get(), dex_cache.Get(), hiddenapi::kLinking)) {
resolved = nullptr;
}
if (resolved != nullptr) {
diff --git a/runtime/dex/art_dex_file_loader.cc b/runtime/dex/art_dex_file_loader.cc
index 9802c69..f3e6a69 100644
--- a/runtime/dex/art_dex_file_loader.cc
+++ b/runtime/dex/art_dex_file_loader.cc
@@ -22,6 +22,7 @@
#include "android-base/stringprintf.h"
#include "base/file_magic.h"
+#include "base/file_utils.h"
#include "base/stl_util.h"
#include "base/systrace.h"
#include "base/unix_file/fd_file.h"
@@ -505,4 +506,39 @@
}
}
+std::unique_ptr<DexFile> ArtDexFileLoader::OpenCommon(const uint8_t* base,
+ size_t size,
+ const uint8_t* data_base,
+ size_t data_size,
+ const std::string& location,
+ uint32_t location_checksum,
+ const OatDexFile* oat_dex_file,
+ bool verify,
+ bool verify_checksum,
+ std::string* error_msg,
+ std::unique_ptr<DexFileContainer> container,
+ VerifyResult* verify_result) {
+ std::unique_ptr<DexFile> dex_file = DexFileLoader::OpenCommon(base,
+ size,
+ data_base,
+ data_size,
+ location,
+ location_checksum,
+ oat_dex_file,
+ verify,
+ verify_checksum,
+ error_msg,
+ std::move(container),
+ verify_result);
+
+ // Check if this dex file is located in the framework directory.
+ // If it is, set a flag on the dex file. This is used by hidden API
+ // policy decision logic.
+ if (dex_file != nullptr && LocationIsOnSystemFramework(location.c_str())) {
+ dex_file->SetIsPlatformDexFile();
+ }
+
+ return dex_file;
+}
+
} // namespace art
diff --git a/runtime/dex/art_dex_file_loader.h b/runtime/dex/art_dex_file_loader.h
index 7c7a59b..7577945 100644
--- a/runtime/dex/art_dex_file_loader.h
+++ b/runtime/dex/art_dex_file_loader.h
@@ -120,6 +120,19 @@
bool verify_checksum,
std::string* error_msg,
ZipOpenErrorCode* error_code) const;
+
+ static std::unique_ptr<DexFile> OpenCommon(const uint8_t* base,
+ size_t size,
+ const uint8_t* data_base,
+ size_t data_size,
+ const std::string& location,
+ uint32_t location_checksum,
+ const OatDexFile* oat_dex_file,
+ bool verify,
+ bool verify_checksum,
+ std::string* error_msg,
+ std::unique_ptr<DexFileContainer> container,
+ VerifyResult* verify_result);
};
} // namespace art
diff --git a/runtime/dex/art_dex_file_loader_test.cc b/runtime/dex/art_dex_file_loader_test.cc
index 6e2cfec..3e0d666 100644
--- a/runtime/dex/art_dex_file_loader_test.cc
+++ b/runtime/dex/art_dex_file_loader_test.cc
@@ -16,9 +16,11 @@
#include <sys/mman.h>
+#include <fstream>
#include <memory>
#include "art_dex_file_loader.h"
+#include "base/file_utils.h"
#include "base/os.h"
#include "base/stl_util.h"
#include "base/unix_file/fd_file.h"
@@ -35,7 +37,40 @@
namespace art {
-class ArtDexFileLoaderTest : public CommonRuntimeTest {};
+static void Copy(const std::string& src, const std::string& dst) {
+ std::ifstream src_stream(src, std::ios::binary);
+ std::ofstream dst_stream(dst, std::ios::binary);
+ dst_stream << src_stream.rdbuf();
+}
+
+class ArtDexFileLoaderTest : public CommonRuntimeTest {
+ public:
+ virtual void SetUp() {
+ CommonRuntimeTest::SetUp();
+
+ std::string dex_location = GetTestDexFileName("Main");
+
+ data_location_path_ = android_data_ + "/foo.jar";
+ system_location_path_ = GetAndroidRoot() + "/foo.jar";
+ system_framework_location_path_ = GetAndroidRoot() + "/framework/foo.jar";
+
+ Copy(dex_location, data_location_path_);
+ Copy(dex_location, system_location_path_);
+ Copy(dex_location, system_framework_location_path_);
+ }
+
+ virtual void TearDown() {
+ remove(data_location_path_.c_str());
+ remove(system_location_path_.c_str());
+ remove(system_framework_location_path_.c_str());
+ CommonRuntimeTest::TearDown();
+ }
+
+ protected:
+ std::string data_location_path_;
+ std::string system_location_path_;
+ std::string system_framework_location_path_;
+};
// TODO: Port OpenTestDexFile(s) need to be ported to use non-ART utilities, and
// the tests that depend upon them should be moved to dex_file_loader_test.cc
@@ -304,4 +339,57 @@
ASSERT_EQ(0, unlink(dex_location_sym.c_str()));
}
+TEST_F(ArtDexFileLoaderTest, IsPlatformDexFile) {
+ ArtDexFileLoader loader;
+ bool success;
+ std::string error_msg;
+ std::vector<std::unique_ptr<const DexFile>> dex_files;
+
+ // Load file from a non-system directory and check that it is not flagged as framework.
+ ASSERT_FALSE(LocationIsOnSystemFramework(data_location_path_.c_str()));
+ success = loader.Open(data_location_path_.c_str(),
+ data_location_path_,
+ /* verify */ false,
+ /* verify_checksum */ false,
+ &error_msg,
+ &dex_files);
+ ASSERT_TRUE(success);
+ ASSERT_GE(dex_files.size(), 1u);
+ for (std::unique_ptr<const DexFile>& dex_file : dex_files) {
+ ASSERT_FALSE(dex_file->IsPlatformDexFile());
+ }
+
+ dex_files.clear();
+
+ // Load file from a system, non-framework directory and check that it is not flagged as framework.
+ ASSERT_FALSE(LocationIsOnSystemFramework(system_location_path_.c_str()));
+ success = loader.Open(system_location_path_.c_str(),
+ system_location_path_,
+ /* verify */ false,
+ /* verify_checksum */ false,
+ &error_msg,
+ &dex_files);
+ ASSERT_TRUE(success);
+ ASSERT_GE(dex_files.size(), 1u);
+ for (std::unique_ptr<const DexFile>& dex_file : dex_files) {
+ ASSERT_FALSE(dex_file->IsPlatformDexFile());
+ }
+
+ dex_files.clear();
+
+ // Load file from a system/framework directory and check that it is flagged as a framework dex.
+ ASSERT_TRUE(LocationIsOnSystemFramework(system_framework_location_path_.c_str()));
+ success = loader.Open(system_framework_location_path_.c_str(),
+ system_framework_location_path_,
+ /* verify */ false,
+ /* verify_checksum */ false,
+ &error_msg,
+ &dex_files);
+ ASSERT_TRUE(success);
+ ASSERT_GE(dex_files.size(), 1u);
+ for (std::unique_ptr<const DexFile>& dex_file : dex_files) {
+ ASSERT_TRUE(dex_file->IsPlatformDexFile());
+ }
+}
+
} // namespace art
diff --git a/runtime/hidden_api.h b/runtime/hidden_api.h
index 5c6b4b5..dbe776e 100644
--- a/runtime/hidden_api.h
+++ b/runtime/hidden_api.h
@@ -130,15 +130,15 @@
}
// Returns true if access to `member` should be denied to the caller of the
-// reflective query. The decision is based on whether the caller is in boot
-// class path or not. Because different users of this function determine this
-// in a different way, `fn_caller_in_boot(self)` is called and should return
-// true if the caller is in boot class path.
+// reflective query. The decision is based on whether the caller is in the
+// platform or not. Because different users of this function determine this
+// in a different way, `fn_caller_in_platform(self)` is called and should
+// return true if the caller is located in the platform.
// This function might print warnings into the log if the member is hidden.
template<typename T>
inline bool ShouldBlockAccessToMember(T* member,
Thread* self,
- std::function<bool(Thread*)> fn_caller_in_boot,
+ std::function<bool(Thread*)> fn_caller_in_platform,
AccessMethod access_method)
REQUIRES_SHARED(Locks::mutator_lock_) {
DCHECK(member != nullptr);
@@ -149,14 +149,14 @@
return false;
}
- // Member is hidden. Walk the stack to find the caller.
+ // Member is hidden. Invoke `fn_caller_in_platform` and find the origin of the access.
// This can be *very* expensive. Save it for last.
- if (fn_caller_in_boot(self)) {
- // Caller in boot class path. Exit.
+ if (fn_caller_in_platform(self)) {
+ // Caller in the platform. Exit.
return false;
}
- // Member is hidden and we are not in the boot class path.
+ // Member is hidden and caller is not in the platform.
// Print a log message with information about this class member access.
// We do this regardless of whether we block the access or not.
@@ -187,18 +187,39 @@
return false;
}
+// Returns true if the caller is either loaded by the boot strap class loader or comes from
+// a dex file located in ${ANDROID_ROOT}/framework/.
+inline bool IsCallerInPlatformDex(ObjPtr<mirror::ClassLoader> caller_class_loader,
+ ObjPtr<mirror::DexCache> caller_dex_cache)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ if (caller_class_loader.IsNull()) {
+ return true;
+ } else if (caller_dex_cache.IsNull()) {
+ return false;
+ } else {
+ const DexFile* caller_dex_file = caller_dex_cache->GetDexFile();
+ return caller_dex_file != nullptr && caller_dex_file->IsPlatformDexFile();
+ }
+}
+
+inline bool IsCallerInPlatformDex(ObjPtr<mirror::Class> caller)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ return !caller.IsNull() && IsCallerInPlatformDex(caller->GetClassLoader(), caller->GetDexCache());
+}
+
// Returns true if access to `member` should be denied to a caller loaded with
// `caller_class_loader`.
// This function might print warnings into the log if the member is hidden.
template<typename T>
inline bool ShouldBlockAccessToMember(T* member,
ObjPtr<mirror::ClassLoader> caller_class_loader,
+ ObjPtr<mirror::DexCache> caller_dex_cache,
AccessMethod access_method)
REQUIRES_SHARED(Locks::mutator_lock_) {
- bool caller_in_boot = (caller_class_loader.IsNull());
+ bool caller_in_platform = IsCallerInPlatformDex(caller_class_loader, caller_dex_cache);
return ShouldBlockAccessToMember(member,
/* thread */ nullptr,
- [caller_in_boot] (Thread*) { return caller_in_boot; },
+ [caller_in_platform] (Thread*) { return caller_in_platform; },
access_method);
}
diff --git a/runtime/interpreter/unstarted_runtime.cc b/runtime/interpreter/unstarted_runtime.cc
index 76df65f..4a2dd3b 100644
--- a/runtime/interpreter/unstarted_runtime.cc
+++ b/runtime/interpreter/unstarted_runtime.cc
@@ -184,6 +184,7 @@
return hiddenapi::ShouldBlockAccessToMember(
member,
frame->GetMethod()->GetDeclaringClass()->GetClassLoader(),
+ frame->GetMethod()->GetDeclaringClass()->GetDexCache(),
hiddenapi::kReflection); // all uses in this file are from reflection
}
diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc
index b78fcac..f309581 100644
--- a/runtime/jni_internal.cc
+++ b/runtime/jni_internal.cc
@@ -80,18 +80,15 @@
// things not rendering correctly. E.g. b/16858794
static constexpr bool kWarnJniAbort = false;
-static bool IsCallerInBootClassPath(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_) {
- ObjPtr<mirror::Class> klass = GetCallingClass(self, /* num_frames */ 1);
- // If `klass` is null, it is an unattached native thread. Assume this is
- // *not* boot class path.
- return klass != nullptr && klass->IsBootStrapClassLoaded();
+static bool IsCallerInPlatformDex(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_) {
+ return hiddenapi::IsCallerInPlatformDex(GetCallingClass(self, /* num_frames */ 1));
}
template<typename T>
ALWAYS_INLINE static bool ShouldBlockAccessToMember(T* member, Thread* self)
REQUIRES_SHARED(Locks::mutator_lock_) {
return hiddenapi::ShouldBlockAccessToMember(
- member, self, IsCallerInBootClassPath, hiddenapi::kJNI);
+ member, self, IsCallerInPlatformDex, hiddenapi::kJNI);
}
// Helpers to call instrumentation functions for fields. These take jobjects so we don't need to set
diff --git a/runtime/native/java_lang_Class.cc b/runtime/native/java_lang_Class.cc
index fc61c95..ad05856 100644
--- a/runtime/native/java_lang_Class.cc
+++ b/runtime/native/java_lang_Class.cc
@@ -49,8 +49,8 @@
namespace art {
-// Returns true if the first non-ClassClass caller up the stack is in boot class path.
-static bool IsCallerInBootClassPath(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_) {
+// Returns true if the first non-ClassClass caller up the stack is in a platform dex file.
+static bool IsCallerInPlatformDex(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_) {
// Walk the stack and find the first frame not from java.lang.Class.
// This is very expensive. Save this till the last.
struct FirstNonClassClassCallerVisitor : public StackVisitor {
@@ -82,7 +82,7 @@
FirstNonClassClassCallerVisitor visitor(self);
visitor.WalkStack();
return visitor.caller != nullptr &&
- visitor.caller->GetDeclaringClass()->IsBootStrapClassLoaded();
+ hiddenapi::IsCallerInPlatformDex(visitor.caller->GetDeclaringClass());
}
// Returns true if the first non-ClassClass caller up the stack is not allowed to
@@ -90,7 +90,7 @@
ALWAYS_INLINE static bool ShouldEnforceHiddenApi(Thread* self)
REQUIRES_SHARED(Locks::mutator_lock_) {
hiddenapi::EnforcementPolicy policy = Runtime::Current()->GetHiddenApiEnforcementPolicy();
- return policy != hiddenapi::EnforcementPolicy::kNoChecks && !IsCallerInBootClassPath(self);
+ return policy != hiddenapi::EnforcementPolicy::kNoChecks && !IsCallerInPlatformDex(self);
}
// Returns true if the first non-ClassClass caller up the stack should not be
@@ -99,7 +99,7 @@
ALWAYS_INLINE static bool ShouldBlockAccessToMember(T* member, Thread* self)
REQUIRES_SHARED(Locks::mutator_lock_) {
return hiddenapi::ShouldBlockAccessToMember(
- member, self, IsCallerInBootClassPath, hiddenapi::kReflection);
+ member, self, IsCallerInPlatformDex, hiddenapi::kReflection);
}
// Returns true if a class member should be discoverable with reflection given