From 2e0aad2d3d66694d1a4ef890653b2c373dab54e3 Mon Sep 17 00:00:00 2001 From: Michal Karpinski Date: Fri, 12 Apr 2019 16:22:55 +0100 Subject: [PATCH] If the calling process isn't whitelisted for background activity start, check the other processes of calling UID too Since looking at whitelisted processes can be computationally more expensive than other checks in the barrier, move them to be last - if callingUid is allowed to open bg activity via other means, like permission, we'll bail quicker, especially for apps like gmscore. Bug: 129853314 Test: atest WmTests:ActivityStarterTests Test: atest BackgroundActivityLaunchTest Test: atest CtsActivityManagerDeviceTestCases:ActivityStarterTests Test: atest WmTests:WindowProcessControllerMapTests Test: manual (repro the quoted bug, and see that while com.google.android.googlequicksearchbox:search isn't whitelisted, AGSA had other whitelisting process, com.google.android.googlequicksearchbox:interactor) Change-Id: Ic7d6c12e76cb707f06e0f216c9958d5d2f21a840 --- .../android/server/wm/ActivityStarter.java | 63 ++++----- .../server/wm/ActivityTaskManagerService.java | 27 ++-- .../android/server/wm/CompatModePackages.java | 6 +- .../server/wm/WindowProcessController.java | 43 ++++-- .../server/wm/WindowProcessControllerMap.java | 86 ++++++++++++ .../wm/WindowProcessControllerMapTests.java | 130 ++++++++++++++++++ 6 files changed, 289 insertions(+), 66 deletions(-) create mode 100644 services/core/java/com/android/server/wm/WindowProcessControllerMap.java create mode 100644 services/tests/wmtests/src/com/android/server/wm/WindowProcessControllerMapTests.java diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java index 895744424ee1f..c8fcb9ea6b297 100644 --- a/services/core/java/com/android/server/wm/ActivityStarter.java +++ b/services/core/java/com/android/server/wm/ActivityStarter.java @@ -980,31 +980,6 @@ class ActivityStarter { return false; } } - // If we don't have callerApp at this point, no caller was provided to startActivity(). - // That's the case for PendingIntent-based starts, since the creator's process might not be - // up and alive. If that's the case, we retrieve the WindowProcessController for the send() - // caller, so that we can make the decision based on its foreground/whitelisted state. - if (callerApp == null) { - callerApp = mService.getProcessController(realCallingPid, realCallingUid); - } - if (callerApp != null) { - // don't abort if the callerApp is instrumenting with background activity starts privs - if (callerApp.isInstrumentingWithBackgroundActivityStartPrivileges()) { - return false; - } - // don't abort if the caller is currently temporarily whitelisted - if (callerApp.areBackgroundActivityStartsAllowed()) { - return false; - } - // don't abort if the caller has an activity in any foreground task - if (callerApp.hasActivityInVisibleTask()) { - return false; - } - // don't abort if the caller is bound by a UID that's currently foreground - if (isBoundByForegroundUid(callerApp)) { - return false; - } - } // don't abort if the callingUid has START_ACTIVITIES_FROM_BACKGROUND permission if (mService.checkPermission(START_ACTIVITIES_FROM_BACKGROUND, callingPid, callingUid) == PERMISSION_GRANTED) { @@ -1029,6 +1004,33 @@ class ActivityStarter { + " temporarily whitelisted. This will not be supported in future Q builds."); return false; } + // If we don't have callerApp at this point, no caller was provided to startActivity(). + // That's the case for PendingIntent-based starts, since the creator's process might not be + // up and alive. If that's the case, we retrieve the WindowProcessController for the send() + // caller, so that we can make the decision based on its foreground/whitelisted state. + int callerAppUid = callingUid; + if (callerApp == null) { + callerApp = mService.getProcessController(realCallingPid, realCallingUid); + callerAppUid = realCallingUid; + } + // don't abort if the callerApp or other processes of that uid are whitelisted in any way + if (callerApp != null) { + // first check the original calling process + if (callerApp.areBackgroundActivityStartsAllowed()) { + return false; + } + // only if that one wasn't whitelisted, check the other ones + final ArraySet uidProcesses = + mService.mProcessMap.getProcesses(callerAppUid); + if (uidProcesses != null) { + for (int i = uidProcesses.size() - 1; i >= 0; i--) { + final WindowProcessController proc = uidProcesses.valueAt(i); + if (proc != callerApp && proc.areBackgroundActivityStartsAllowed()) { + return false; + } + } + } + } // anything that has fallen through would currently be aborted Slog.w(TAG, "Background activity start [callingPackage: " + callingPackage + "; callingUid: " + callingUid @@ -1053,17 +1055,6 @@ class ActivityStarter { return true; } - private boolean isBoundByForegroundUid(WindowProcessController callerApp) { - final ArraySet boundClientUids = callerApp.getBoundClientUids(); - for (int i = boundClientUids.size() - 1; i >= 0; --i) { - final int uid = boundClientUids.valueAt(i); - if (mService.isUidForeground(uid)) { - return true; - } - } - return false; - } - // TODO: remove this toast after feature development is done void showBackgroundActivityBlockedToast(boolean abort, String callingPackage) { final Resources res = mService.mContext.getResources(); diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index 76774d73cc035..3fa0268739537 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -373,8 +373,8 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { private final SparseArray mPendingTempWhitelist = new SparseArray<>(); /** All processes currently running that might have a window organized by name. */ final ProcessMap mProcessNames = new ProcessMap<>(); - /** All processes we currently have running mapped by pid */ - final SparseArray mPidMap = new SparseArray<>(); + /** All processes we currently have running mapped by pid and uid */ + final WindowProcessControllerMap mProcessMap = new WindowProcessControllerMap(); /** This is the process holding what we currently consider to be the "home" activity. */ WindowProcessController mHomeProcess; /** The currently running heavy-weight process, if any. */ @@ -913,7 +913,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { return getGlobalConfiguration(); } synchronized (mGlobalLock) { - final WindowProcessController app = mPidMap.get(pid); + final WindowProcessController app = mProcessMap.getProcess(pid); return app != null ? app.getConfiguration() : getGlobalConfiguration(); } } @@ -4640,7 +4640,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { enforceSystemHasVrFeature(); synchronized (mGlobalLock) { final int pid = Binder.getCallingPid(); - final WindowProcessController wpc = mPidMap.get(pid); + final WindowProcessController wpc = mProcessMap.getProcess(pid); mVrController.setVrThreadLocked(tid, pid, wpc); } } @@ -4659,7 +4659,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { enforceSystemHasVrFeature(); synchronized (mGlobalLock) { final int pid = Binder.getCallingPid(); - final WindowProcessController proc = mPidMap.get(pid); + final WindowProcessController proc = mProcessMap.getProcess(pid); mVrController.setPersistentVrThreadLocked(tid, pid, proc); } } @@ -5204,9 +5204,10 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { mH.sendMessage(msg); } - for (int i = mPidMap.size() - 1; i >= 0; i--) { - final int pid = mPidMap.keyAt(i); - final WindowProcessController app = mPidMap.get(pid); + SparseArray pidMap = mProcessMap.getPidMap(); + for (int i = pidMap.size() - 1; i >= 0; i--) { + final int pid = pidMap.keyAt(i); + final WindowProcessController app = pidMap.get(pid); if (DEBUG_CONFIGURATION) { Slog.v(TAG_CONFIGURATION, "Update process config of " + app.mName + " to new config " + configCopy); @@ -5859,7 +5860,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { } WindowProcessController getProcessController(int pid, int uid) { - final WindowProcessController proc = mPidMap.get(pid); + final WindowProcessController proc = mProcessMap.getProcess(pid); if (proc == null) return null; if (UserHandle.isApp(uid) && proc.mUid == uid) { return proc; @@ -6423,14 +6424,14 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { @Override public void onProcessMapped(int pid, WindowProcessController proc) { synchronized (mGlobalLock) { - mPidMap.put(pid, proc); + mProcessMap.put(pid, proc); } } @Override public void onProcessUnMapped(int pid) { synchronized (mGlobalLock) { - mPidMap.remove(pid); + mProcessMap.remove(pid); } } @@ -6503,7 +6504,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { } return; } - final WindowProcessController process = mPidMap.get(pid); + final WindowProcessController process = mProcessMap.getProcess(pid); if (process == null) { if (DEBUG_CONFIGURATION) { Slog.w(TAG, "Trying to update display configuration for invalid " @@ -6696,7 +6697,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { // Only allow this from foreground processes, so that background // applications can't abuse it to prevent system UI from being shown. if (uid >= FIRST_APPLICATION_UID) { - final WindowProcessController proc = mPidMap.get(pid); + final WindowProcessController proc = mProcessMap.getProcess(pid); if (!proc.isPerceptible()) { Slog.w(TAG, "Ignoring closeSystemDialogs " + reason + " from background process " + proc); diff --git a/services/core/java/com/android/server/wm/CompatModePackages.java b/services/core/java/com/android/server/wm/CompatModePackages.java index c8f8e82bdb181..104805fba3082 100644 --- a/services/core/java/com/android/server/wm/CompatModePackages.java +++ b/services/core/java/com/android/server/wm/CompatModePackages.java @@ -48,6 +48,7 @@ import android.os.Message; import android.os.RemoteException; import android.util.AtomicFile; import android.util.Slog; +import android.util.SparseArray; import android.util.Xml; public final class CompatModePackages { @@ -324,8 +325,9 @@ public final class CompatModePackages { ActivityRecord starting = stack.restartPackage(packageName); // Tell all processes that loaded this package about the change. - for (int i = mService.mPidMap.size() - 1; i >= 0; i--) { - final WindowProcessController app = mService.mPidMap.valueAt(i); + SparseArray pidMap = mService.mProcessMap.getPidMap(); + for (int i = pidMap.size() - 1; i >= 0; i--) { + final WindowProcessController app = pidMap.valueAt(i); if (!app.mPkgList.contains(packageName)) { continue; } diff --git a/services/core/java/com/android/server/wm/WindowProcessController.java b/services/core/java/com/android/server/wm/WindowProcessController.java index 4ca35f7d427bf..eb919eb00f0c4 100644 --- a/services/core/java/com/android/server/wm/WindowProcessController.java +++ b/services/core/java/com/android/server/wm/WindowProcessController.java @@ -372,18 +372,39 @@ public class WindowProcessController extends ConfigurationContainer= 0; --i) { + if (mAtm.isUidForeground(mBoundClientUids.valueAt(i))) { + return true; + } + } + return false; } public void setBoundClientUids(ArraySet boundClientUids) { mBoundClientUids = boundClientUids; } - public ArraySet getBoundClientUids() { - return mBoundClientUids; - } - public void setInstrumenting(boolean instrumenting, boolean hasBackgroundActivityStartPrivileges) { mInstrumenting = instrumenting; @@ -394,14 +415,6 @@ public class WindowProcessController extends ConfigurationContainer= 0; --i) { TaskRecord task = mActivities.get(i).getTaskRecord(); if (task == null) { diff --git a/services/core/java/com/android/server/wm/WindowProcessControllerMap.java b/services/core/java/com/android/server/wm/WindowProcessControllerMap.java new file mode 100644 index 0000000000000..2767972f7ea01 --- /dev/null +++ b/services/core/java/com/android/server/wm/WindowProcessControllerMap.java @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.wm; + +import android.util.ArraySet; +import android.util.SparseArray; + +import java.util.Map; +import java.util.HashMap; + +final class WindowProcessControllerMap { + + /** All processes we currently have running mapped by pid */ + private final SparseArray mPidMap = new SparseArray<>(); + /** All processes we currently have running mapped by uid */ + private final Map> mUidMap = new HashMap<>(); + + /** Retrieves a currently running process for pid. */ + WindowProcessController getProcess(int pid) { + return mPidMap.get(pid); + } + + /** Retrieves all currently running processes for uid. */ + ArraySet getProcesses(int uid) { + return mUidMap.get(uid); + } + + SparseArray getPidMap() { + return mPidMap; + } + + void put(int pid, WindowProcessController proc) { + // if there is a process for this pid already in mPidMap it'll get replaced automagically, + // but we actually need to remove it from mUidMap too before adding the new one + final WindowProcessController prevProc = mPidMap.get(pid); + if (prevProc != null) { + removeProcessFromUidMap(prevProc); + } + // put process into mPidMap + mPidMap.put(pid, proc); + // put process into mUidMap + final int uid = proc.mUid; + ArraySet procSet = mUidMap.getOrDefault(uid, + new ArraySet()); + procSet.add(proc); + mUidMap.put(uid, procSet); + } + + void remove(int pid) { + final WindowProcessController proc = mPidMap.get(pid); + if (proc != null) { + // remove process from mPidMap + mPidMap.remove(pid); + // remove process from mUidMap + removeProcessFromUidMap(proc); + } + } + + private void removeProcessFromUidMap(WindowProcessController proc) { + if (proc == null) { + return; + } + final int uid = proc.mUid; + ArraySet procSet = mUidMap.get(uid); + if (procSet != null) { + procSet.remove(proc); + if (procSet.isEmpty()) { + mUidMap.remove(uid); + } + } + } +} diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowProcessControllerMapTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowProcessControllerMapTests.java new file mode 100644 index 0000000000000..cb7bff3a4f155 --- /dev/null +++ b/services/tests/wmtests/src/com/android/server/wm/WindowProcessControllerMapTests.java @@ -0,0 +1,130 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package com.android.server.wm; + +import static com.android.dx.mockito.inline.extended.ExtendedMockito.mock; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import android.os.UserHandle; +import android.platform.test.annotations.Presubmit; +import android.util.ArraySet; + +import androidx.test.filters.SmallTest; + +import org.junit.Before; +import org.junit.Test; + +/** + * Tests for the {@link WindowProcessControllerMap} class. + * + * Build/Install/Run: + * atest WmTests:WindowProcessControllerMapTests + */ +@SmallTest +@Presubmit +public class WindowProcessControllerMapTests extends ActivityTestsBase { + + private static final int FAKE_UID1 = 666; + private static final int FAKE_UID2 = 667; + private static final int FAKE_PID1 = 668; + private static final int FAKE_PID2 = 669; + private static final int FAKE_PID3 = 670; + private static final int FAKE_PID4 = 671; + + private WindowProcessControllerMap mProcessMap; + private WindowProcessController pid1uid1; + private WindowProcessController pid1uid2; + private WindowProcessController pid2uid1; + private WindowProcessController pid3uid1; + private WindowProcessController pid4uid2; + + @Before + public void setUp() throws Exception { + mProcessMap = new WindowProcessControllerMap(); + pid1uid1 = new WindowProcessController( + mService, mService.mContext.getApplicationInfo(), "fakepid1fakeuid1", FAKE_UID1, + UserHandle.getUserId(12345), mock(Object.class), mock(WindowProcessListener.class)); + pid1uid1.setPid(FAKE_PID1); + pid1uid2 = new WindowProcessController( + mService, mService.mContext.getApplicationInfo(), "fakepid1fakeuid2", FAKE_UID2, + UserHandle.getUserId(12345), mock(Object.class), mock(WindowProcessListener.class)); + pid1uid2.setPid(FAKE_PID1); + pid2uid1 = new WindowProcessController( + mService, mService.mContext.getApplicationInfo(), "fakepid2fakeuid1", FAKE_UID1, + UserHandle.getUserId(12345), mock(Object.class), mock(WindowProcessListener.class)); + pid2uid1.setPid(FAKE_PID2); + pid3uid1 = new WindowProcessController( + mService, mService.mContext.getApplicationInfo(), "fakepid3fakeuid1", FAKE_UID1, + UserHandle.getUserId(12345), mock(Object.class), mock(WindowProcessListener.class)); + pid3uid1.setPid(FAKE_PID3); + pid4uid2 = new WindowProcessController( + mService, mService.mContext.getApplicationInfo(), "fakepid4fakeuid2", FAKE_UID2, + UserHandle.getUserId(12345), mock(Object.class), mock(WindowProcessListener.class)); + pid4uid2.setPid(FAKE_PID4); + } + + @Test + public void testAdditionsAndRemovals() { + // test various additions and removals + mProcessMap.put(FAKE_PID1, pid1uid1); + mProcessMap.put(FAKE_PID2, pid2uid1); + assertEquals(pid1uid1, mProcessMap.getProcess(FAKE_PID1)); + assertEquals(pid2uid1, mProcessMap.getProcess(FAKE_PID2)); + ArraySet uid1processes = mProcessMap.getProcesses(FAKE_UID1); + assertTrue(uid1processes.contains(pid1uid1)); + assertTrue(uid1processes.contains(pid2uid1)); + assertEquals(uid1processes.size(), 2); + + mProcessMap.remove(FAKE_PID2); + mProcessMap.put(FAKE_PID3, pid3uid1); + uid1processes = mProcessMap.getProcesses(FAKE_UID1); + assertTrue(uid1processes.contains(pid1uid1)); + assertFalse(uid1processes.contains(pid2uid1)); + assertTrue(uid1processes.contains(pid3uid1)); + assertEquals(uid1processes.size(), 2); + + mProcessMap.put(FAKE_PID4, pid4uid2); + ArraySet uid2processes = mProcessMap.getProcesses(FAKE_UID2); + assertTrue(uid2processes.contains(pid4uid2)); + assertEquals(uid2processes.size(), 1); + + mProcessMap.remove(FAKE_PID1); + mProcessMap.remove(FAKE_PID3); + assertNull(mProcessMap.getProcesses(FAKE_UID1)); + assertEquals(mProcessMap.getProcess(FAKE_PID4), pid4uid2); + } + + @Test + public void testReplacement() { + // test that replacing a process is handled correctly + mProcessMap.put(FAKE_PID1, pid1uid1); + ArraySet uid1processes = mProcessMap.getProcesses(FAKE_UID1); + assertTrue(uid1processes.contains(pid1uid1)); + assertEquals(uid1processes.size(), 1); + + mProcessMap.put(FAKE_PID1, pid1uid2); + assertNull(mProcessMap.getProcesses(FAKE_UID1)); + ArraySet uid2processes = mProcessMap.getProcesses(FAKE_UID2); + assertTrue(uid2processes.contains(pid1uid2)); + assertEquals(uid2processes.size(), 1); + assertEquals(mProcessMap.getProcess(FAKE_PID1), pid1uid2); + } +}