diff --git a/services/core/java/com/android/server/power/hint/HintManagerService.java b/services/core/java/com/android/server/power/hint/HintManagerService.java index fc7628c28b8b1..6014d0cee1711 100644 --- a/services/core/java/com/android/server/power/hint/HintManagerService.java +++ b/services/core/java/com/android/server/power/hint/HintManagerService.java @@ -17,6 +17,7 @@ package com.android.server.power.hint; import android.app.ActivityManager; +import android.app.ActivityManagerInternal; import android.app.IUidObserver; import android.content.Context; import android.os.Binder; @@ -33,12 +34,16 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.DumpUtils; import com.android.internal.util.Preconditions; import com.android.server.FgThread; +import com.android.server.LocalServices; import com.android.server.SystemService; import com.android.server.utils.Slogf; import java.io.FileDescriptor; import java.io.PrintWriter; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; +import java.util.Objects; /** An hint service implementation that runs in System Server process. */ public final class HintManagerService extends SystemService { @@ -56,6 +61,8 @@ public final class HintManagerService extends SystemService { private final NativeWrapper mNativeWrapper; + private final ActivityManagerInternal mAmInternal; + @VisibleForTesting final IHintManager.Stub mService = new BinderService(); public HintManagerService(Context context) { @@ -70,6 +77,8 @@ public final class HintManagerService extends SystemService { mNativeWrapper.halInit(); mHintSessionPreferredRate = mNativeWrapper.halGetHintSessionPreferredRate(); mUidObserver = new UidObserver(); + mAmInternal = Objects.requireNonNull( + LocalServices.getService(ActivityManagerInternal.class)); } @VisibleForTesting @@ -85,7 +94,7 @@ public final class HintManagerService extends SystemService { @Override public void onStart() { - publishBinderService(Context.PERFORMANCE_HINT_SERVICE, mService, /* allowIsolated= */ true); + publishBinderService(Context.PERFORMANCE_HINT_SERVICE, mService); } @Override @@ -243,12 +252,30 @@ public final class HintManagerService extends SystemService { return mService; } - private boolean checkTidValid(int tgid, int [] tids) { - // Make sure all tids belongs to the same process. + private boolean checkTidValid(int uid, int tgid, int [] tids) { + // Make sure all tids belongs to the same UID (including isolated UID), + // tids can belong to different application processes. + List eligiblePids = mAmInternal.getIsolatedProcesses(uid); + if (eligiblePids == null) { + eligiblePids = new ArrayList<>(); + } + eligiblePids.add(tgid); + for (int threadId : tids) { - if (!Process.isThreadInProcess(tgid, threadId)) { - return false; + final String[] procStatusKeys = new String[] { + "Uid:", + "Tgid:" + }; + long[] output = new long[procStatusKeys.length]; + Process.readProcLines("/proc/" + threadId + "/status", procStatusKeys, output); + int uidOfThreadId = (int) output[0]; + int pidOfThreadId = (int) output[1]; + + // use PID check for isolated processes, use UID check for non-isolated processes. + if (eligiblePids.contains(pidOfThreadId) || uidOfThreadId == uid) { + continue; } + return false; } return true; } @@ -264,27 +291,25 @@ public final class HintManagerService extends SystemService { Preconditions.checkArgument(tids.length != 0, "tids should" + " not be empty."); - int uid = Binder.getCallingUid(); - int tid = Binder.getCallingPid(); - int pid = Process.getThreadGroupLeader(tid); - + final int callingUid = Binder.getCallingUid(); + final int callingTgid = Process.getThreadGroupLeader(Binder.getCallingPid()); final long identity = Binder.clearCallingIdentity(); try { - if (!checkTidValid(pid, tids)) { - throw new SecurityException("Some tid doesn't belong to the process"); + if (!checkTidValid(callingUid, callingTgid, tids)) { + throw new SecurityException("Some tid doesn't belong to the application"); } - long halSessionPtr = mNativeWrapper.halCreateHintSession(pid, uid, tids, - durationNanos); + long halSessionPtr = mNativeWrapper.halCreateHintSession(callingTgid, callingUid, + tids, durationNanos); if (halSessionPtr == 0) return null; - AppHintSession hs = new AppHintSession(uid, pid, tids, token, + AppHintSession hs = new AppHintSession(callingUid, callingTgid, tids, token, halSessionPtr, durationNanos); synchronized (mLock) { - ArrayMap tokenMap = mActiveSessions.get(uid); + ArrayMap tokenMap = mActiveSessions.get(callingUid); if (tokenMap == null) { tokenMap = new ArrayMap<>(1); - mActiveSessions.put(uid, tokenMap); + mActiveSessions.put(callingUid, tokenMap); } tokenMap.put(token, hs); return hs; diff --git a/services/tests/servicestests/src/com/android/server/power/hint/HintManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/power/hint/HintManagerServiceTest.java index aaf40d7e44617..397770bec822d 100644 --- a/services/tests/servicestests/src/com/android/server/power/hint/HintManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/power/hint/HintManagerServiceTest.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyLong; @@ -36,6 +37,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.ActivityManager; +import android.app.ActivityManagerInternal; import android.content.Context; import android.os.Binder; import android.os.IBinder; @@ -43,6 +45,7 @@ import android.os.IHintSession; import android.os.Process; import com.android.server.FgThread; +import com.android.server.LocalServices; import com.android.server.power.hint.HintManagerService.AppHintSession; import com.android.server.power.hint.HintManagerService.Injector; import com.android.server.power.hint.HintManagerService.NativeWrapper; @@ -74,6 +77,7 @@ public class HintManagerServiceTest { @Mock private Context mContext; @Mock private HintManagerService.NativeWrapper mNativeWrapperMock; + @Mock private ActivityManagerInternal mAmInternalMock; private HintManagerService mService; @@ -86,6 +90,9 @@ public class HintManagerServiceTest { eq(DEFAULT_TARGET_DURATION))).thenReturn(1L); when(mNativeWrapperMock.halCreateHintSession(eq(TGID), eq(UID), eq(SESSION_TIDS_B), eq(DEFAULT_TARGET_DURATION))).thenReturn(2L); + when(mAmInternalMock.getIsolatedProcesses(anyInt())).thenReturn(null); + LocalServices.removeServiceForTest(ActivityManagerInternal.class); + LocalServices.addService(ActivityManagerInternal.class, mAmInternalMock); } private HintManagerService createService() { @@ -104,6 +111,17 @@ public class HintManagerServiceTest { assertThat(service.mHintSessionPreferredRate).isEqualTo(DEFAULT_HINT_PREFERRED_RATE); } + @Test + public void testCreateHintSessionInvalidPid() throws Exception { + HintManagerService service = createService(); + IBinder token = new Binder(); + // Make sure we throw exception when adding a TID doesn't belong to the processes + // In this case, we add `init` PID into the list. + assertThrows(SecurityException.class, + () -> service.getBinderServiceInstance().createHintSession(token, + new int[]{TID, 1}, DEFAULT_TARGET_DURATION)); + } + @Test public void testCreateHintSession() throws Exception { HintManagerService service = createService();