From d979e1d76fa00d6eccb2d254fea7b258867fb4b2 Mon Sep 17 00:00:00 2001 From: Bernardo Rufino Date: Thu, 8 Apr 2021 14:51:36 +0100 Subject: [PATCH] Handle PendingIntents for ACSD Finsky for some reason sends Intent.ACSD in notification trampolines by calling PendingIntent.getBroadcast(...).send() instead of Context.sendBroadcast(). The problem is that because it's using a PI, the callingPid is righfully -1 (since the process could have been killed or even restarted w/ a different pid by the time the PI is fired), so we don't allow by the trampoline exemption for Intent.ACSD for targetSdk < S. In order to support that use-case, we need to look up the current processes for the given UID, which removes the need to receive a WindowProcessController altogether. Bug: 183274733 Test: atest -d CtsAppTestCases:android.app.cts.CloseSystemDialogsTest Change-Id: I0c1f21a21f1dac311cea5f6276c84e590b2cc0ef --- .../server/wm/ActivityTaskManagerService.java | 59 +++++++++---------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index 7ae42cc9396a5..3dcc03760aebe 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -2937,7 +2937,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { if (packageName != null) { caller = packageName + " " + caller; } - if (!canCloseSystemDialogs(pid, uid, process)) { + if (!canCloseSystemDialogs(pid, uid)) { // The app can't close system dialogs, throw only if it targets S+ if (CompatChanges.isChangeEnabled(LOCK_DOWN_CLOSE_SYSTEM_DIALOGS, uid)) { throw new SecurityException( @@ -2962,39 +2962,37 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { return true; } - private boolean canCloseSystemDialogs(int pid, int uid, - @Nullable WindowProcessController process) { + private boolean canCloseSystemDialogs(int pid, int uid) { if (checkPermission(Manifest.permission.BROADCAST_CLOSE_SYSTEM_DIALOGS, pid, uid) == PERMISSION_GRANTED) { return true; } - if (process == null) { - synchronized (mGlobalLock) { - process = mProcessMap.getProcess(pid); + synchronized (mGlobalLock) { + // Check all the processes from the given uid, especially since for PendingIntents sent + // the pid equals -1 + ArraySet processes = mProcessMap.getProcesses(uid); + if (processes != null) { + for (int i = 0, n = processes.size(); i < n; i++) { + WindowProcessController process = processes.valueAt(i); + // Check if the instrumentation of the process has the permission. This covers + // the usual test started from the shell (which has the permission) case. This + // is needed for apps targeting SDK level < S but we are also allowing for + // targetSdk S+ as a convenience to avoid breaking a bunch of existing tests and + // asking them to adopt shell permissions to do this. + int sourceUid = process.getInstrumentationSourceUid(); + if (process.isInstrumenting() && sourceUid != -1 && checkPermission( + Manifest.permission.BROADCAST_CLOSE_SYSTEM_DIALOGS, -1, sourceUid) + == PERMISSION_GRANTED) { + return true; + } + // This is the notification trampoline use-case for example, where apps use + // Intent.ACSD to close the shade prior to starting an activity. + if (process.canCloseSystemDialogsByToken()) { + return true; + } + } } - } - if (process != null) { - // Check if the instrumentation of the process has the permission. This covers the - // usual test started from the shell (which has the permission) case. This is needed - // for apps targeting SDK level < S but we are also allowing for targetSdk S+ as a - // convenience to avoid breaking a bunch of existing tests and asking them to adopt - // shell permissions to do this. - // Note that these getters all read from volatile fields in WindowProcessController, so - // no need to lock. - int sourceUid = process.getInstrumentationSourceUid(); - if (process.isInstrumenting() && sourceUid != -1 && checkPermission( - Manifest.permission.BROADCAST_CLOSE_SYSTEM_DIALOGS, -1, sourceUid) - == PERMISSION_GRANTED) { - return true; - } - // This is the notification trampoline use-case for example, where apps use Intent.ACSD - // to close the shade prior to starting an activity. - if (process.canCloseSystemDialogsByToken()) { - return true; - } - } - if (!CompatChanges.isChangeEnabled(LOCK_DOWN_CLOSE_SYSTEM_DIALOGS, uid)) { - synchronized (mGlobalLock) { + if (!CompatChanges.isChangeEnabled(LOCK_DOWN_CLOSE_SYSTEM_DIALOGS, uid)) { // This covers the case where the app is displaying some UI on top of the // notification shade and wants to start an activity. The app then sends the intent // in order to move the notification shade out of the way and show the activity to @@ -5308,8 +5306,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { @Override public boolean canCloseSystemDialogs(int pid, int uid) { - return ActivityTaskManagerService.this.canCloseSystemDialogs(pid, uid, - null /* process */); + return ActivityTaskManagerService.this.canCloseSystemDialogs(pid, uid); } @Override