Fix the setThreads JNI to throw SecurityException
Bug: 291830812
Test: atest FrameworksCoreTests:PerformanceHintManagerTest PerformanceHintTest
Change-Id: I35b28772c1ad2cf26e9f9f7f019406ef7c15303e
diff --git a/core/java/android/os/PerformanceHintManager.java b/core/java/android/os/PerformanceHintManager.java
index f79d6e6..977ef60 100644
--- a/core/java/android/os/PerformanceHintManager.java
+++ b/core/java/android/os/PerformanceHintManager.java
@@ -29,6 +29,7 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.ref.Reference;
+import java.util.Objects;
/** The PerformanceHintManager allows apps to send performance hint to system. */
@@ -239,6 +240,7 @@
if (mNativeSessionPtr == 0) {
return;
}
+ Objects.requireNonNull(tids, "tids cannot be null");
if (tids.length == 0) {
throw new IllegalArgumentException("Thread id list can't be empty.");
}
diff --git a/core/jni/android_os_PerformanceHintManager.cpp b/core/jni/android_os_PerformanceHintManager.cpp
index d8a2497..ffe844d 100644
--- a/core/jni/android_os_PerformanceHintManager.cpp
+++ b/core/jni/android_os_PerformanceHintManager.cpp
@@ -41,7 +41,7 @@
typedef void (*APH_reportActualWorkDuration)(APerformanceHintSession*, int64_t);
typedef void (*APH_closeSession)(APerformanceHintSession* session);
typedef void (*APH_sendHint)(APerformanceHintSession*, int32_t);
-typedef void (*APH_setThreads)(APerformanceHintSession*, const pid_t*, size_t);
+typedef int (*APH_setThreads)(APerformanceHintSession*, const pid_t*, size_t);
typedef void (*APH_getThreadIds)(APerformanceHintSession*, int32_t* const, size_t* const);
bool gAPerformanceHintBindingInitialized = false;
@@ -112,6 +112,20 @@
} // namespace
+static void throwExceptionForErrno(JNIEnv* env, int err, const std::string& msg) {
+ switch (err) {
+ case EINVAL:
+ jniThrowExceptionFmt(env, "java/lang/IllegalArgumentException", msg.c_str());
+ break;
+ case EPERM:
+ jniThrowExceptionFmt(env, "java/lang/SecurityException", msg.c_str());
+ break;
+ default:
+ jniThrowException(env, "java/lang/RuntimeException", msg.c_str());
+ break;
+ }
+}
+
static jlong nativeAcquireManager(JNIEnv* env, jclass clazz) {
ensureAPerformanceHintBindingInitialized();
return reinterpret_cast<jlong>(gAPH_getManagerFn());
@@ -174,8 +188,11 @@
for (size_t i = 0; i < tidsArray.size(); ++i) {
tidsVector.push_back(static_cast<int32_t>(tidsArray[i]));
}
- gAPH_setThreadsFn(reinterpret_cast<APerformanceHintSession*>(nativeSessionPtr),
- tidsVector.data(), tidsVector.size());
+ int err = gAPH_setThreadsFn(reinterpret_cast<APerformanceHintSession*>(nativeSessionPtr),
+ tidsVector.data(), tidsVector.size());
+ if (err != 0) {
+ throwExceptionForErrno(env, err, "Failed to set threads for hint session");
+ }
}
// This call should only be used for validation in tests only. This call will initiate two IPC
diff --git a/core/tests/coretests/src/android/os/PerformanceHintManagerTest.java b/core/tests/coretests/src/android/os/PerformanceHintManagerTest.java
index 7eefbbc..2c03fdc 100644
--- a/core/tests/coretests/src/android/os/PerformanceHintManagerTest.java
+++ b/core/tests/coretests/src/android/os/PerformanceHintManagerTest.java
@@ -139,11 +139,20 @@
}
@Test
- public void testSetThreadsWithIllegalArgument() {
+ public void testSetThreads_emptyTids() {
Session session = createSession();
assumeNotNull(session);
assertThrows(IllegalArgumentException.class, () -> {
- session.setThreads(new int[] { });
+ session.setThreads(new int[]{});
+ });
+ }
+
+ @Test
+ public void testSetThreads_invalidTids() {
+ Session session = createSession();
+ assumeNotNull(session);
+ assertThrows(SecurityException.class, () -> {
+ session.setThreads(new int[]{-1});
});
}
}
diff --git a/native/android/performance_hint.cpp b/native/android/performance_hint.cpp
index b3628fa..6198f40 100644
--- a/native/android/performance_hint.cpp
+++ b/native/android/performance_hint.cpp
@@ -274,9 +274,10 @@
binder::Status ret = mHintManager->setHintSessionThreads(mHintSession, tids);
if (!ret.isOk()) {
ALOGE("%s: failed: %s", __FUNCTION__, ret.exceptionMessage().c_str());
- if (ret.exceptionCode() == binder::Status::Exception::EX_SECURITY ||
- ret.exceptionCode() == binder::Status::Exception::EX_ILLEGAL_ARGUMENT) {
+ if (ret.exceptionCode() == binder::Status::Exception::EX_ILLEGAL_ARGUMENT) {
return EINVAL;
+ } else if (ret.exceptionCode() == binder::Status::Exception::EX_SECURITY) {
+ return EPERM;
}
return EPIPE;
}
diff --git a/native/android/tests/performance_hint/PerformanceHintNativeTest.cpp b/native/android/tests/performance_hint/PerformanceHintNativeTest.cpp
index 791adfd..6f7562b 100644
--- a/native/android/tests/performance_hint/PerformanceHintNativeTest.cpp
+++ b/native/android/tests/performance_hint/PerformanceHintNativeTest.cpp
@@ -178,4 +178,15 @@
.WillOnce(Return(Status()));
result = APerformanceHint_setThreads(session, newTids.data(), newTids.size());
EXPECT_EQ(0, result);
+
+ testing::Mock::VerifyAndClearExpectations(mMockIHintManager);
+ std::vector<int32_t> invalidTids;
+ auto status = Status::fromExceptionCode(binder::Status::Exception::EX_SECURITY);
+ invalidTids.push_back(4);
+ invalidTids.push_back(6);
+ EXPECT_CALL(*mMockIHintManager, setHintSessionThreads(_, Eq(invalidTids)))
+ .Times(Exactly(1))
+ .WillOnce(Return(status));
+ result = APerformanceHint_setThreads(session, invalidTids.data(), invalidTids.size());
+ EXPECT_EQ(EPERM, result);
}