Start warning on hidden API greylist
Insert checks into reflection, JNI and the verifier to print
a warning when greylisted methods are invoked and fields accessed.
We do this on actual access, because reflection allows to list
all methods/fields and simply listing a greylisted member would
print too many false positives.
Issuing a warning also sets a boolean flag in Runtime. This will
be made accessible through VMRuntime to the framework which will
issue a Toast on Activity start.
The change was tested with internal microbenchmarks of reflection
and those flagged one issue. Microbenchmark invoking a field getter
has regressed by 35%. We will profile this benchmark in detail and
consider options for improvement. Bug b/72482474 was created to track
progress.
Test: art/test.py -b -r -t 674-hiddenapi
Bug: 64382372
Bug: 72482474
Change-Id: I323244935e9091a2f8d012385cefaac6b1fe3777
diff --git a/runtime/hidden_api.h b/runtime/hidden_api.h
index 241850e..de3a51a 100644
--- a/runtime/hidden_api.h
+++ b/runtime/hidden_api.h
@@ -41,6 +41,28 @@
}
}
+// Returns true if we should warn about non-boot class path accessing member
+// with `access_flags`.
+inline bool ShouldWarnAboutMember(uint32_t access_flags) {
+ if (!Runtime::Current()->AreHiddenApiChecksEnabled()) {
+ return false;
+ }
+
+ switch (HiddenApiAccessFlags::DecodeFromRuntime(access_flags)) {
+ case HiddenApiAccessFlags::kWhitelist:
+ return false;
+ case HiddenApiAccessFlags::kLightGreylist:
+ case HiddenApiAccessFlags::kDarkGreylist:
+ return true;
+ case HiddenApiAccessFlags::kBlacklist:
+ // We should never access a blacklisted member from non-boot class path,
+ // but this function is called before we establish the origin of the access.
+ // Return false here, we do not want to warn when boot class path accesses
+ // a blacklisted member.
+ return false;
+ }
+}
+
// Returns true if caller `num_frames` up the stack is in boot class path.
inline bool IsCallerInBootClassPath(Thread* self, size_t num_frames)
REQUIRES_SHARED(Locks::mutator_lock_) {
@@ -78,6 +100,70 @@
!IsCallerInBootClassPath(self, num_frames); // This is expensive. Save it for last.
}
+// Issue a warning about field access.
+inline void WarnAboutMemberAccess(ArtField* field) REQUIRES_SHARED(Locks::mutator_lock_) {
+ Runtime::Current()->SetPendingHiddenApiWarning(true);
+ LOG(WARNING) << "Access to hidden field " << field->PrettyField();
+}
+
+// Issue a warning about method access.
+inline void WarnAboutMemberAccess(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
+ Runtime::Current()->SetPendingHiddenApiWarning(true);
+ LOG(WARNING) << "Access to hidden method " << method->PrettyMethod();
+}
+
+// Set access flags of `member` to be in hidden API whitelist. This can be disabled
+// with a Runtime::SetDedupHiddenApiWarnings.
+template<typename T>
+inline void MaybeWhitelistMember(T* member) REQUIRES_SHARED(Locks::mutator_lock_) {
+ if (Runtime::Current()->ShouldDedupeHiddenApiWarnings()) {
+ member->SetAccessFlags(HiddenApiAccessFlags::EncodeForRuntime(
+ member->GetAccessFlags(), HiddenApiAccessFlags::kWhitelist));
+ DCHECK(!ShouldWarnAboutMember(member->GetAccessFlags()));
+ }
+}
+
+// Check if `caller` should be allowed to access `member` and warn if not.
+template<typename T>
+inline void MaybeWarnAboutMemberAccess(T* member, ArtMethod* caller)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ DCHECK(member != nullptr);
+ DCHECK(!caller->IsRuntimeMethod());
+ if (!Runtime::Current()->AreHiddenApiChecksEnabled() ||
+ member == nullptr ||
+ !ShouldWarnAboutMember(member->GetAccessFlags()) ||
+ caller->GetDeclaringClass()->IsBootStrapClassLoaded()) {
+ return;
+ }
+
+ WarnAboutMember(member);
+ MaybeWhitelistMember(member);
+}
+
+// Check if the caller `num_frames` up the stack should be allowed to access
+// `member` and warn if not.
+template<typename T>
+inline void MaybeWarnAboutMemberAccess(T* member, Thread* self, size_t num_frames)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ if (!Runtime::Current()->AreHiddenApiChecksEnabled() ||
+ member == nullptr ||
+ !ShouldWarnAboutMember(member->GetAccessFlags())) {
+ return;
+ }
+
+ // Walk the stack to find the caller. This is *very* expensive. Save it for last.
+ ObjPtr<mirror::Class> klass = GetCallingClass(self, num_frames);
+ if (klass == nullptr) {
+ // Unattached native thread, assume that this is *not* boot class path
+ // and enforce the rules.
+ } else if (klass->IsBootStrapClassLoaded()) {
+ return;
+ }
+
+ WarnAboutMemberAccess(member);
+ MaybeWhitelistMember(member);
+}
+
} // namespace hiddenapi
} // namespace art
diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc
index 727f54b..c40360f 100644
--- a/runtime/jni_internal.cc
+++ b/runtime/jni_internal.cc
@@ -80,15 +80,17 @@
// things not rendering correctly. E.g. b/16858794
static constexpr bool kWarnJniAbort = false;
-// Helpers to call instrumentation functions for fields. These take jobjects so we don't need to set
-// up handles for the rare case where these actually do something. Once these functions return it is
-// possible there will be a pending exception if the instrumentation happens to throw one.
+// Helpers to check if we need to warn about accessing hidden API fields and to call instrumentation
+// functions for them. These take jobjects so we don't need to set up handles for the rare case
+// where these actually do something. Once these functions return it is possible there will be
+// a pending exception if the instrumentation happens to throw one.
static void NotifySetObjectField(ArtField* field, jobject obj, jobject jval)
REQUIRES_SHARED(Locks::mutator_lock_) {
DCHECK_EQ(field->GetTypeAsPrimitiveType(), Primitive::kPrimNot);
+ Thread* self = Thread::Current();
+ hiddenapi::MaybeWarnAboutMemberAccess(field, self, /* num_frames */ 1);
instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
if (UNLIKELY(instrumentation->HasFieldWriteListeners())) {
- Thread* self = Thread::Current();
ArtMethod* cur_method = self->GetCurrentMethod(/*dex_pc*/ nullptr,
/*check_suspended*/ true,
/*abort_on_error*/ false);
@@ -113,9 +115,10 @@
static void NotifySetPrimitiveField(ArtField* field, jobject obj, JValue val)
REQUIRES_SHARED(Locks::mutator_lock_) {
DCHECK_NE(field->GetTypeAsPrimitiveType(), Primitive::kPrimNot);
+ Thread* self = Thread::Current();
+ hiddenapi::MaybeWarnAboutMemberAccess(field, self, /* num_frames */ 1);
instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
if (UNLIKELY(instrumentation->HasFieldWriteListeners())) {
- Thread* self = Thread::Current();
ArtMethod* cur_method = self->GetCurrentMethod(/*dex_pc*/ nullptr,
/*check_suspended*/ true,
/*abort_on_error*/ false);
@@ -137,9 +140,10 @@
static void NotifyGetField(ArtField* field, jobject obj)
REQUIRES_SHARED(Locks::mutator_lock_) {
+ Thread* self = Thread::Current();
+ hiddenapi::MaybeWarnAboutMemberAccess(field, self, /* num_frames */ 1);
instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
if (UNLIKELY(instrumentation->HasFieldReadListeners())) {
- Thread* self = Thread::Current();
ArtMethod* cur_method = self->GetCurrentMethod(/*dex_pc*/ nullptr,
/*check_suspended*/ true,
/*abort_on_error*/ false);
diff --git a/runtime/native/java_lang_Class.cc b/runtime/native/java_lang_Class.cc
index 4d36e80..5544275 100644
--- a/runtime/native/java_lang_Class.cc
+++ b/runtime/native/java_lang_Class.cc
@@ -831,6 +831,7 @@
return nullptr;
}
}
+ hiddenapi::MaybeWarnAboutMemberAccess(constructor, soa.Self(), /* num_frames */ 1);
// Invoke the constructor.
JValue result;
uint32_t args[1] = { static_cast<uint32_t>(reinterpret_cast<uintptr_t>(receiver.Get())) };
diff --git a/runtime/native/java_lang_reflect_Field.cc b/runtime/native/java_lang_reflect_Field.cc
index f990c04..db7f4bb 100644
--- a/runtime/native/java_lang_reflect_Field.cc
+++ b/runtime/native/java_lang_reflect_Field.cc
@@ -25,6 +25,7 @@
#include "common_throws.h"
#include "dex/dex_file-inl.h"
#include "dex/dex_file_annotations.h"
+#include "hidden_api.h"
#include "jni_internal.h"
#include "mirror/class-inl.h"
#include "mirror/field-inl.h"
@@ -161,6 +162,9 @@
DCHECK(soa.Self()->IsExceptionPending());
return nullptr;
}
+
+ hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1);
+
// We now don't expect suspension unless an exception is thrown.
// Get the field's value, boxing if necessary.
Primitive::Type field_type = f->GetTypeAsPrimitiveType();
@@ -183,13 +187,14 @@
DCHECK(soa.Self()->IsExceptionPending());
return JValue();
}
-
// If field is not set to be accessible, verify it can be accessed by the caller.
if (!f->IsAccessible() && !VerifyFieldAccess<false>(soa.Self(), f, o)) {
DCHECK(soa.Self()->IsExceptionPending());
return JValue();
}
+ hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1);
+
// We now don't expect suspension unless an exception is thrown.
// Read the value.
Primitive::Type field_type = f->GetTypeAsPrimitiveType();
@@ -351,11 +356,15 @@
DCHECK(soa.Self()->IsExceptionPending());
return;
}
+
// If field is not set to be accessible, verify it can be accessed by the caller.
if (!f->IsAccessible() && !VerifyFieldAccess<true>(soa.Self(), f, o)) {
DCHECK(soa.Self()->IsExceptionPending());
return;
}
+
+ hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1);
+
SetFieldValue(o, f, field_prim_type, true, unboxed_value);
}
@@ -391,6 +400,8 @@
return;
}
+ hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1);
+
// Write the value.
SetFieldValue(o, f, field_type, false, wide_value);
}
diff --git a/runtime/reflection.cc b/runtime/reflection.cc
index 635a03a..6ffafe0 100644
--- a/runtime/reflection.cc
+++ b/runtime/reflection.cc
@@ -465,6 +465,9 @@
}
ArtMethod* method = jni::DecodeArtMethod(mid);
+
+ hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1);
+
bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
if (is_string_init) {
// Replace calls to String.<init> with equivalent StringFactory call.
@@ -496,6 +499,9 @@
}
ArtMethod* method = jni::DecodeArtMethod(mid);
+
+ hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1);
+
bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
if (is_string_init) {
// Replace calls to String.<init> with equivalent StringFactory call.
@@ -528,6 +534,9 @@
ObjPtr<mirror::Object> receiver = soa.Decode<mirror::Object>(obj);
ArtMethod* method = FindVirtualMethod(receiver, jni::DecodeArtMethod(mid));
+
+ hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1);
+
bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
if (is_string_init) {
// Replace calls to String.<init> with equivalent StringFactory call.
@@ -560,6 +569,9 @@
ObjPtr<mirror::Object> receiver = soa.Decode<mirror::Object>(obj);
ArtMethod* method = FindVirtualMethod(receiver, jni::DecodeArtMethod(mid));
+
+ hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1);
+
bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
if (is_string_init) {
// Replace calls to String.<init> with equivalent StringFactory call.
@@ -604,6 +616,8 @@
}
}
+ hiddenapi::MaybeWarnAboutMemberAccess(m, soa.Self(), num_frames);
+
ObjPtr<mirror::Object> receiver;
if (!m->IsStatic()) {
// Replace calls to String.<init> with equivalent StringFactory call.
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 33bebe0..6d065d6 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -266,6 +266,8 @@
is_low_memory_mode_(false),
safe_mode_(false),
do_hidden_api_checks_(false),
+ pending_hidden_api_warning_(false),
+ dedupe_hidden_api_warnings_(true),
dump_native_stack_on_sig_quit_(true),
pruned_dalvik_cache_(false),
// Initially assume we perceive jank in case the process state is never updated.
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 893ebbe..184e4e5 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -528,6 +528,22 @@
return do_hidden_api_checks_;
}
+ void SetPendingHiddenApiWarning(bool value) {
+ pending_hidden_api_warning_ = value;
+ }
+
+ bool HasPendingHiddenApiWarning() const {
+ return pending_hidden_api_warning_;
+ }
+
+ void SetDedupeHiddenApiWarnings(bool value) {
+ dedupe_hidden_api_warnings_ = value;
+ }
+
+ bool ShouldDedupeHiddenApiWarnings() {
+ return dedupe_hidden_api_warnings_;
+ }
+
bool IsDexFileFallbackEnabled() const {
return allow_dex_file_fallback_;
}
@@ -968,6 +984,14 @@
// Whether access checks on hidden API should be performed.
bool do_hidden_api_checks_;
+ // Whether the application has used an API which is not restricted but we
+ // should issue a warning about it.
+ bool pending_hidden_api_warning_;
+
+ // Do not warn about the same hidden API access violation twice.
+ // This is only used for testing.
+ bool dedupe_hidden_api_warnings_;
+
// Whether threads should dump their native stack on SIGQUIT.
bool dump_native_stack_on_sig_quit_;
diff --git a/test/674-hiddenapi/hiddenapi.cc b/test/674-hiddenapi/hiddenapi.cc
index 89cf68c..baff6f7 100644
--- a/test/674-hiddenapi/hiddenapi.cc
+++ b/test/674-hiddenapi/hiddenapi.cc
@@ -27,6 +27,7 @@
extern "C" JNIEXPORT void JNICALL Java_Main_init(JNIEnv*, jclass) {
Runtime::Current()->SetHiddenApiChecksEnabled(true);
+ Runtime::Current()->SetDedupeHiddenApiWarnings(false);
}
extern "C" JNIEXPORT void JNICALL Java_Main_appendToBootClassLoader(
@@ -284,11 +285,11 @@
}
extern "C" JNIEXPORT jboolean JNICALL Java_ChildClass_hasPendingWarning(JNIEnv*, jclass) {
- return false;
+ return Runtime::Current()->HasPendingHiddenApiWarning();
}
extern "C" JNIEXPORT void JNICALL Java_ChildClass_clearWarning(JNIEnv*, jclass) {
- return;
+ Runtime::Current()->SetPendingHiddenApiWarning(false);
}
} // namespace Test674HiddenApi
diff --git a/test/674-hiddenapi/src-ex/ChildClass.java b/test/674-hiddenapi/src-ex/ChildClass.java
index be2a352..babd883 100644
--- a/test/674-hiddenapi/src-ex/ChildClass.java
+++ b/test/674-hiddenapi/src-ex/ChildClass.java
@@ -103,7 +103,7 @@
} else if (hiddenness == Hiddenness.Blacklist) {
expected = Behaviour.Denied;
} else {
- expected = Behaviour.Granted;
+ expected = Behaviour.Warning;
}
for (boolean isStatic : booleanValues) {
@@ -389,7 +389,7 @@
private static void checkLinking(String className, boolean takesParameter, Behaviour behaviour)
throws Exception {
boolean canAccess = (behaviour != Behaviour.Denied);
- boolean setsWarning = (behaviour == Behaviour.Warning);
+ boolean setsWarning = false; // we do not set the flag in verifier or at runtime
clearWarning();
if (Linking.canAccess(className, takesParameter) != canAccess) {