From 7bdc21cf3eb56c3ec16f22fae0c0d20432f328b5 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Mon, 27 Sep 2021 18:27:49 +0800 Subject: [PATCH] Do not lock when calling startActivityFromRecents When calling ActivityStarter#execute without known activity info, it will need to resolve the intent and check content provider uri permission. That may hit potential deadlock (acquire AM lock) so it will return denial directly if the WM global lock is held. Bug: 189057104 Test: Start and stop recording, tap the notification to view the recorded result. Press back to finish the activity. Launch the activity from recents app without permission denial. Test: Pin apps to split from recents. Change-Id: Ibf3fee1f29f3069f0adfa85ed983f6d48bf7ebd7 --- .../server/wm/ActivityTaskManagerService.java | 6 +- .../server/wm/ActivityTaskSupervisor.java | 160 ++++++++++-------- .../com/android/server/wm/AppTaskImpl.java | 10 +- .../server/wm/WindowOrganizerController.java | 31 +++- 4 files changed, 123 insertions(+), 84 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index 681ff9022e4b1..6e5ba31b3ff69 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -1732,10 +1732,8 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { final SafeActivityOptions safeOptions = SafeActivityOptions.fromBundle(bOptions); final long origId = Binder.clearCallingIdentity(); try { - synchronized (mGlobalLock) { - return mTaskSupervisor.startActivityFromRecents(callingPid, callingUid, taskId, - safeOptions); - } + return mTaskSupervisor.startActivityFromRecents(callingPid, callingUid, taskId, + safeOptions); } finally { Binder.restoreCallingIdentity(origId); } diff --git a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java index 0497fb5001d47..145bdfda3ce30 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java +++ b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java @@ -2474,102 +2474,126 @@ public class ActivityTaskSupervisor implements RecentTasks.Callbacks { } } + /** + * Start the given task from the recent tasks. Do not hold WM global lock when calling this + * method to avoid potential deadlock or permission deny by UriGrantsManager when resolving + * activity (see {@link ActivityStarter.Request#resolveActivity} and + * {@link com.android.server.am.ContentProviderHelper#checkContentProviderUriPermission}). + * + * @return The result code of starter. + */ int startActivityFromRecents(int callingPid, int callingUid, int taskId, SafeActivityOptions options) { - Task task = null; + final Task task; + final int taskCallingUid; final String callingPackage; final String callingFeatureId; final Intent intent; final int userId; - int activityType = ACTIVITY_TYPE_UNDEFINED; - int windowingMode = WINDOWING_MODE_UNDEFINED; final ActivityOptions activityOptions = options != null ? options.getOptions(this) : null; boolean moveHomeTaskForward = true; - if (activityOptions != null) { - activityType = activityOptions.getLaunchActivityType(); - windowingMode = activityOptions.getLaunchWindowingMode(); - if (activityOptions.freezeRecentTasksReordering() - && mRecentTasks.isCallerRecents(callingUid)) { - mRecentTasks.setFreezeTaskListReordering(); + synchronized (mService.mGlobalLock) { + int activityType = ACTIVITY_TYPE_UNDEFINED; + if (activityOptions != null) { + activityType = activityOptions.getLaunchActivityType(); + final int windowingMode = activityOptions.getLaunchWindowingMode(); + if (activityOptions.freezeRecentTasksReordering() + && mRecentTasks.isCallerRecents(callingUid)) { + mRecentTasks.setFreezeTaskListReordering(); + } + if (windowingMode == WINDOWING_MODE_SPLIT_SCREEN_PRIMARY + || activityOptions.getLaunchRootTask() != null) { + // Don't move home activity forward if we are launching into primary split or + // there is a launch root set. + moveHomeTaskForward = false; + } } - if (windowingMode == WINDOWING_MODE_SPLIT_SCREEN_PRIMARY - || activityOptions.getLaunchRootTask() != null) { - // Don't move home activity forward if we are launching into primary split or there - // is a launch root set. - moveHomeTaskForward = false; - } - } - if (activityType == ACTIVITY_TYPE_HOME || activityType == ACTIVITY_TYPE_RECENTS) { - throw new IllegalArgumentException("startActivityFromRecents: Task " - + taskId + " can't be launch in the home/recents root task."); - } - - mService.deferWindowLayout(); - try { - task = mRootWindowContainer.anyTaskForId(taskId, - MATCH_ATTACHED_TASK_OR_RECENT_TASKS_AND_RESTORE, activityOptions, ON_TOP); - if (task == null) { - mWindowManager.executeAppTransition(); - throw new IllegalArgumentException( - "startActivityFromRecents: Task " + taskId + " not found."); + if (activityType == ACTIVITY_TYPE_HOME || activityType == ACTIVITY_TYPE_RECENTS) { + throw new IllegalArgumentException("startActivityFromRecents: Task " + + taskId + " can't be launch in the home/recents root task."); } - if (moveHomeTaskForward) { - // We always want to return to the home activity instead of the recents activity - // from whatever is started from the recents activity, so move the home root task - // forward. - // TODO (b/115289124): Multi-display supports for recents. - mRootWindowContainer.getDefaultTaskDisplayArea().moveHomeRootTaskToFront( - "startActivityFromRecents"); - } - - // If the user must confirm credentials (e.g. when first launching a work app and the - // Work Challenge is present) let startActivityInPackage handle the intercepting. - if (!mService.mAmInternal.shouldConfirmCredentials(task.mUserId) - && task.getRootActivity() != null) { - final ActivityRecord targetActivity = task.getTopNonFinishingActivity(); - - mRootWindowContainer.startPowerModeLaunchIfNeeded( - true /* forceSend */, targetActivity); - final LaunchingState launchingState = - mActivityMetricsLogger.notifyActivityLaunching(task.intent); - try { - mService.moveTaskToFrontLocked(null /* appThread */, null /* callingPackage */, - task.mTaskId, 0, options); - // Apply options to prevent pendingOptions be taken when scheduling activity - // lifecycle transaction to make sure the override pending app transition will - // be applied immediately. - targetActivity.applyOptionsAnimation(); - } finally { - mActivityMetricsLogger.notifyActivityLaunched(launchingState, - START_TASK_TO_FRONT, false /* newActivityCreated */, targetActivity, - activityOptions); + boolean shouldStartActivity = false; + mService.deferWindowLayout(); + try { + task = mRootWindowContainer.anyTaskForId(taskId, + MATCH_ATTACHED_TASK_OR_RECENT_TASKS_AND_RESTORE, activityOptions, ON_TOP); + if (task == null) { + mWindowManager.executeAppTransition(); + throw new IllegalArgumentException( + "startActivityFromRecents: Task " + taskId + " not found."); } - mService.getActivityStartController().postStartActivityProcessingForLastStarter( - task.getTopNonFinishingActivity(), ActivityManager.START_TASK_TO_FRONT, - task.getRootTask()); + if (moveHomeTaskForward) { + // We always want to return to the home activity instead of the recents + // activity from whatever is started from the recents activity, so move + // the home root task forward. + // TODO (b/115289124): Multi-display supports for recents. + mRootWindowContainer.getDefaultTaskDisplayArea().moveHomeRootTaskToFront( + "startActivityFromRecents"); + } - // As it doesn't go to ActivityStarter.executeRequest() path, we need to resume - // app switching here also. - mService.resumeAppSwitches(); + // If the user must confirm credentials (e.g. when first launching a work + // app and the Work Challenge is present) let startActivityInPackage handle + // the intercepting. + if (!mService.mAmInternal.shouldConfirmCredentials(task.mUserId) + && task.getRootActivity() != null) { + final ActivityRecord targetActivity = task.getTopNonFinishingActivity(); - return ActivityManager.START_TASK_TO_FRONT; + mRootWindowContainer.startPowerModeLaunchIfNeeded( + true /* forceSend */, targetActivity); + final LaunchingState launchingState = + mActivityMetricsLogger.notifyActivityLaunching(task.intent); + try { + mService.moveTaskToFrontLocked(null /* appThread */, + null /* callingPackage */, task.mTaskId, 0, options); + // Apply options to prevent pendingOptions be taken when scheduling + // activity lifecycle transaction to make sure the override pending app + // transition will be applied immediately. + targetActivity.applyOptionsAnimation(); + } finally { + mActivityMetricsLogger.notifyActivityLaunched(launchingState, + START_TASK_TO_FRONT, false /* newActivityCreated */, + targetActivity, activityOptions); + } + + mService.getActivityStartController().postStartActivityProcessingForLastStarter( + task.getTopNonFinishingActivity(), ActivityManager.START_TASK_TO_FRONT, + task.getRootTask()); + + // As it doesn't go to ActivityStarter.executeRequest() path, we need to resume + // app switching here also. + mService.resumeAppSwitches(); + return ActivityManager.START_TASK_TO_FRONT; + } + // The task is empty or needs to show the confirmation for credential. + shouldStartActivity = true; + } finally { + if (!shouldStartActivity) { + mService.continueWindowLayout(); + } } + taskCallingUid = task.mCallingUid; callingPackage = task.mCallingPackage; callingFeatureId = task.mCallingFeatureId; intent = task.intent; intent.addFlags(Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY); userId = task.mUserId; - return mService.getActivityStartController().startActivityInPackage(task.mCallingUid, + } + // ActivityStarter will acquire the lock where the places need, so execute the request + // outside of the lock. + try { + return mService.getActivityStartController().startActivityInPackage(taskCallingUid, callingPid, callingUid, callingPackage, callingFeatureId, intent, null, null, null, 0, 0, options, userId, task, "startActivityFromRecents", false /* validateIncomingUser */, null /* originatingPendingIntent */, false /* allowBackgroundActivityStart */); } finally { - mService.continueWindowLayout(); + synchronized (mService.mGlobalLock) { + mService.continueWindowLayout(); + } } } diff --git a/services/core/java/com/android/server/wm/AppTaskImpl.java b/services/core/java/com/android/server/wm/AppTaskImpl.java index 7f0adcacc9513..5589396119054 100644 --- a/services/core/java/com/android/server/wm/AppTaskImpl.java +++ b/services/core/java/com/android/server/wm/AppTaskImpl.java @@ -35,10 +35,10 @@ import android.os.UserHandle; */ class AppTaskImpl extends IAppTask.Stub { private static final String TAG = "AppTaskImpl"; - private ActivityTaskManagerService mService; + private final ActivityTaskManagerService mService; - private int mTaskId; - private int mCallingUid; + private final int mTaskId; + private final int mCallingUid; public AppTaskImpl(ActivityTaskManagerService service, int taskId, int callingUid) { mService = service; @@ -113,9 +113,9 @@ class AppTaskImpl extends IAppTask.Stub { return; } } - mService.mTaskSupervisor.startActivityFromRecents(callingPid, - callingUid, mTaskId, null); } + mService.mTaskSupervisor.startActivityFromRecents(callingPid, callingUid, mTaskId, + null /* options */); } finally { Binder.restoreCallingIdentity(origId); } diff --git a/services/core/java/com/android/server/wm/WindowOrganizerController.java b/services/core/java/com/android/server/wm/WindowOrganizerController.java index 4cc764ac33cac..69313810fa931 100644 --- a/services/core/java/com/android/server/wm/WindowOrganizerController.java +++ b/services/core/java/com/android/server/wm/WindowOrganizerController.java @@ -318,7 +318,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub * @param caller Info about the calling process. */ private void applyTransaction(@NonNull WindowContainerTransaction t, int syncId, - @Nullable Transition transition, @Nullable CallerInfo caller) { + @Nullable Transition transition, @NonNull CallerInfo caller) { int effects = 0; ProtoLog.v(WM_DEBUG_WINDOW_ORGANIZER, "Apply window transaction, syncId=%d", syncId); mService.deferWindowLayout(); @@ -540,7 +540,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub private int applyHierarchyOp(WindowContainerTransaction.HierarchyOp hop, int effects, int syncId, @Nullable Transition transition, boolean isInLockTaskMode, - @Nullable CallerInfo caller, @Nullable IBinder errorCallbackToken, + @NonNull CallerInfo caller, @Nullable IBinder errorCallbackToken, @Nullable ITaskFragmentOrganizer organizer) { final int type = hop.getType(); switch (type) { @@ -628,11 +628,28 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub final int taskId = launchOpts.getInt( WindowContainerTransaction.HierarchyOp.LAUNCH_KEY_TASK_ID); launchOpts.remove(WindowContainerTransaction.HierarchyOp.LAUNCH_KEY_TASK_ID); - final SafeActivityOptions safeOptions = caller != null - ? SafeActivityOptions.fromBundle(launchOpts, caller.mPid, caller.mUid) - : SafeActivityOptions.fromBundle(launchOpts); - mService.mTaskSupervisor.startActivityFromRecents(caller.mPid, caller.mUid, - taskId, safeOptions); + final SafeActivityOptions safeOptions = + SafeActivityOptions.fromBundle(launchOpts, caller.mPid, caller.mUid); + final Integer[] starterResult = { null }; + // startActivityFromRecents should not be called in lock. + mService.mH.post(() -> { + try { + starterResult[0] = mService.mTaskSupervisor.startActivityFromRecents( + caller.mPid, caller.mUid, taskId, safeOptions); + } catch (Throwable t) { + starterResult[0] = ActivityManager.START_CANCELED; + Slog.w(TAG, t); + } + synchronized (mGlobalLock) { + mGlobalLock.notifyAll(); + } + }); + while (starterResult[0] == null) { + try { + mGlobalLock.wait(); + } catch (InterruptedException ignored) { + } + } break; case HIERARCHY_OP_TYPE_PENDING_INTENT: String resolvedType = hop.getActivityIntent() != null