From 67a799c9730f927505ed9d91ca65da0dfc340088 Mon Sep 17 00:00:00 2001 From: Hani Kazmi Date: Fri, 3 Mar 2023 14:30:12 +0000 Subject: [PATCH] ASM - Bug fixes for clearTopIfNeeded This includes the following changes: 1. activity starts with UID 0 could fail - they do not have a corresponding packageName, resulting in a NPE in the feature flag allowlist. We now both explicitely allow UID 0, and check for nulls. 2. We no longer exclude finishing activities in the check. While this is generally a no-op, it resulted in noisy logs/toasts. E.G, an activity finishes itself and launches a new one with NEW_TASK at the same time would result in clearTop being called, even though there is nothing to clear. This is net-neutral for security - the finishing activity could have already morphed itself rather than finishing. 3. We move the home task to front of the finishing tas in checkActivitySecurityForTaskClear, rather than to front of display area. For multi-window scenarios, this could result in one app forcing a home switch for the other. 4. Allow launches from top of stack, even if finishing or alwaysOnTop. This supports tasks which are always set to alwaysOnTop, if they try to start or finish activities Bug: 271417931 Test: ABTD with all WM tests and InputMethod tests Change-Id: I966319723b17f1affd92e141791596e5da644050 --- .../server/wm/ActivitySecurityModelFeatureFlags.java | 5 ++++- .../java/com/android/server/wm/ActivityStarter.java | 9 +++++---- .../com/android/server/wm/ActivityTaskSupervisor.java | 10 ++++++++-- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivitySecurityModelFeatureFlags.java b/services/core/java/com/android/server/wm/ActivitySecurityModelFeatureFlags.java index 1f7af41df6e0e..19d8129bb6063 100644 --- a/services/core/java/com/android/server/wm/ActivitySecurityModelFeatureFlags.java +++ b/services/core/java/com/android/server/wm/ActivitySecurityModelFeatureFlags.java @@ -43,7 +43,7 @@ class ActivitySecurityModelFeatureFlags { static final String DOC_LINK = "go/android-asm"; /** Used to determine which version of the ASM logic was used in logs while we iterate */ - static final int ASM_VERSION = 6; + static final int ASM_VERSION = 7; private static final String NAMESPACE = NAMESPACE_WINDOW_MANAGER; private static final String KEY_ASM_PREFIX = "ActivitySecurity__"; @@ -89,6 +89,9 @@ class ActivitySecurityModelFeatureFlags { if (flagEnabled) { String[] packageNames = sPm.getPackagesForUid(uid); + if (packageNames == null) { + return true; + } for (int i = 0; i < packageNames.length; i++) { if (sExcludedPackageNames.contains(packageNames[i])) { return false; diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java index 7024886156bf6..83f27d4dccfad 100644 --- a/services/core/java/com/android/server/wm/ActivityStarter.java +++ b/services/core/java/com/android/server/wm/ActivityStarter.java @@ -2249,13 +2249,14 @@ class ActivityStarter { */ private void clearTopIfNeeded(@NonNull Task targetTask, int callingUid, int realCallingUid, int startingUid, int launchFlags) { - if ((launchFlags & FLAG_ACTIVITY_NEW_TASK) != FLAG_ACTIVITY_NEW_TASK) { - // Launch is from the same task, so must be a top or privileged UID + if ((launchFlags & FLAG_ACTIVITY_NEW_TASK) != FLAG_ACTIVITY_NEW_TASK + || mBalCode == BAL_ALLOW_ALLOWLISTED_UID) { + // Launch is from the same task, (a top or privileged UID), or is directly privileged. return; } - Predicate isLaunchingOrLaunched = ar -> !ar.finishing - && (ar.isUid(startingUid) || ar.isUid(callingUid) || ar.isUid(realCallingUid)); + Predicate isLaunchingOrLaunched = ar -> + ar.isUid(startingUid) || ar.isUid(callingUid) || ar.isUid(realCallingUid); // Return early if we know for sure we won't need to clear any activities by just checking // the top activity. diff --git a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java index a0a2557392912..eaf55838afe79 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java +++ b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java @@ -1666,7 +1666,7 @@ public class ActivityTaskSupervisor implements RecentTasks.Callbacks { String callerActivityClassName) { // We may have already checked that the callingUid has additional clearTask privileges, and // cleared the calling identify. If so, we infer we do not need further restrictions here. - if (callingUid == SYSTEM_UID) { + if (callingUid == SYSTEM_UID || !task.isVisible() || task.inMultiWindowMode()) { return; } @@ -1772,13 +1772,19 @@ public class ActivityTaskSupervisor implements RecentTasks.Callbacks { return new Pair<>(true, true); } + // Always allow actual top activity to clear task + ActivityRecord topActivity = task.getTopMostActivity(); + if (topActivity != null && topActivity.isUid(uid)) { + return new Pair<>(true, true); + } + // Consider the source activity, whether or not it is finishing. Do not consider any other // finishing activity. Predicate topOfStackPredicate = (ar) -> ar.equals(sourceRecord) || (!ar.finishing && !ar.isAlwaysOnTop()); // Check top of stack (or the first task fragment for embedding). - ActivityRecord topActivity = task.getActivity(topOfStackPredicate); + topActivity = task.getActivity(topOfStackPredicate); if (topActivity == null) { return new Pair<>(false, false); }