diff options
| author | 2019-05-29 14:52:59 -0600 | |
|---|---|---|
| committer | 2019-05-30 11:29:49 -0600 | |
| commit | 344ce7ce703f806f50ea253047d6dbbb10eaa424 (patch) | |
| tree | 6bd74ba11afcc31dcafe1b83f0118f953e191e34 | |
| parent | 190e99cf3ab47b2eb196999aeab92a9eee98cae5 (diff) | |
Collect NeededUriGrants without holding locks.
In preparation for a future security fix, we need to determine Uri
permission grants before acquiring the WM lock. This CL is mostly
mechanical refactoring to split the calculation and granting logic
into two phases, and pass around the calculated NeededUriGrants.
There is no other logic changes included in this CL, so it should
have no effects.
Bug: 115619667
Test: atest WmTests:ActivityStarterTests
Test: atest CtsWindowManagerDeviceTestCases:ActivityStarterTests
Test: atest android.appsecurity.cts.AppSecurityTests
Change-Id: I033e23549008bea26528781f16caa92427b39c71
12 files changed, 218 insertions, 111 deletions
diff --git a/services/core/java/com/android/server/uri/UriGrantsManagerInternal.java b/services/core/java/com/android/server/uri/UriGrantsManagerInternal.java index 2f50fcb08c02..94a1baa49deb 100644 --- a/services/core/java/com/android/server/uri/UriGrantsManagerInternal.java +++ b/services/core/java/com/android/server/uri/UriGrantsManagerInternal.java @@ -43,14 +43,14 @@ public interface UriGrantsManagerInternal { int callingUid, String targetPkg, Uri uri, int modeFlags, int userId); NeededUriGrants checkGrantUriPermissionFromIntent(int callingUid, String targetPkg, Intent intent, int mode, NeededUriGrants needed, int targetUserId); + NeededUriGrants checkGrantUriPermissionFromIntent(int callingUid, + Intent intent, String targetPkg, int targetUserId); /** * Grant Uri permissions from one app to another. This method only extends * permission grants if {@code callingUid} has permission to them. */ void grantUriPermissionFromIntent(int callingUid, String targetPkg, Intent intent, int targetUserId); - void grantUriPermissionFromIntent(int callingUid, - String targetPkg, Intent intent, UriPermissionOwner owner, int targetUserId); void grantUriPermissionUncheckedFromIntent( NeededUriGrants needed, UriPermissionOwner owner); IBinder newUriPermissionOwner(String name); diff --git a/services/core/java/com/android/server/uri/UriGrantsManagerService.java b/services/core/java/com/android/server/uri/UriGrantsManagerService.java index 8b332d271a3a..04f7c7ee8574 100644 --- a/services/core/java/com/android/server/uri/UriGrantsManagerService.java +++ b/services/core/java/com/android/server/uri/UriGrantsManagerService.java @@ -1362,20 +1362,21 @@ public class UriGrantsManagerService extends IUriGrantsManager.Stub { } @Override - public void grantUriPermissionFromIntent(int callingUid, String targetPkg, Intent intent, - int targetUserId) { + public NeededUriGrants checkGrantUriPermissionFromIntent(int callingUid, Intent intent, + String targetPkg, int targetUserId) { synchronized (mLock) { - UriGrantsManagerService.this.grantUriPermissionFromIntent( - callingUid, targetPkg, intent, null, targetUserId); + final int mode = (intent != null) ? intent.getFlags() : 0; + return UriGrantsManagerService.this.checkGrantUriPermissionFromIntent( + callingUid, targetPkg, intent, mode, null, targetUserId); } } @Override public void grantUriPermissionFromIntent(int callingUid, String targetPkg, Intent intent, - UriPermissionOwner owner, int targetUserId) { + int targetUserId) { synchronized (mLock) { UriGrantsManagerService.this.grantUriPermissionFromIntent( - callingUid, targetPkg, intent, owner, targetUserId); + callingUid, targetPkg, intent, null, targetUserId); } } diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index 1344727ab36d..7c4b5711cbe6 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -215,6 +215,7 @@ import com.android.server.AttributeCache; import com.android.server.AttributeCache.Entry; import com.android.server.am.AppTimeTracker; import com.android.server.am.PendingIntentRecord; +import com.android.server.uri.NeededUriGrants; import com.android.server.uri.UriPermissionOwner; import com.android.server.wm.ActivityMetricsLogger.WindowingModeTransitionInfoSnapshot; import com.android.server.wm.ActivityStack.ActivityState; @@ -1599,10 +1600,11 @@ final class ActivityRecord extends ConfigurationContainer { * Deliver a new Intent to an existing activity, so that its onNewIntent() * method will be called at the proper time. */ - final void deliverNewIntentLocked(int callingUid, Intent intent, String referrer) { + final void deliverNewIntentLocked(int callingUid, Intent intent, NeededUriGrants intentGrants, + String referrer) { // The activity now gets access to the data associated with this Intent. - mAtmService.mUgmInternal.grantUriPermissionFromIntent(callingUid, packageName, - intent, getUriPermissionsLocked(), mUserId); + mAtmService.mUgmInternal.grantUriPermissionUncheckedFromIntent(intentGrants, + getUriPermissionsLocked()); final ReferrerIntent rintent = new ReferrerIntent(intent, referrer); boolean unsent = true; final boolean isTopActivityWhileSleeping = isTopRunningActivity() && isSleeping(); diff --git a/services/core/java/com/android/server/wm/ActivityStack.java b/services/core/java/com/android/server/wm/ActivityStack.java index 3d59e66d13ef..1ad5e7c8f4ae 100644 --- a/services/core/java/com/android/server/wm/ActivityStack.java +++ b/services/core/java/com/android/server/wm/ActivityStack.java @@ -162,6 +162,7 @@ import com.android.server.am.ActivityManagerService.ItemMatcher; import com.android.server.am.AppTimeTracker; import com.android.server.am.EventLogTags; import com.android.server.am.PendingIntentRecord; +import com.android.server.uri.NeededUriGrants; import java.io.FileDescriptor; import java.io.PrintWriter; @@ -2780,7 +2781,7 @@ class ActivityStack extends ConfigurationContainer { if (DEBUG_STATES) Slog.d(TAG_STATES, "no-history finish of " + mLastNoHistoryActivity + " on new resume"); requestFinishActivityLocked(mLastNoHistoryActivity.appToken, Activity.RESULT_CANCELED, - null, "resume-no-history", false); + null, null, "resume-no-history", false); mLastNoHistoryActivity = null; } @@ -3014,7 +3015,7 @@ class ActivityStack extends ConfigurationContainer { // If any exception gets thrown, toss away this // activity and try the next one. Slog.w(TAG, "Exception thrown during resume of " + next, e); - requestFinishActivityLocked(next.appToken, Activity.RESULT_CANCELED, null, + requestFinishActivityLocked(next.appToken, Activity.RESULT_CANCELED, null, null, "resume-exception", true); return true; } @@ -3422,7 +3423,7 @@ class ActivityStack extends ConfigurationContainer { if (DEBUG_TASKS) Slog.w(TAG_TASKS, "resetTaskIntendedTask: calling finishActivity on " + p); if (finishActivityLocked( - p, Activity.RESULT_CANCELED, null, "reset-task", false)) { + p, Activity.RESULT_CANCELED, null, null, "reset-task", false)) { end--; srcPos--; } @@ -3501,7 +3502,7 @@ class ActivityStack extends ConfigurationContainer { continue; } finishActivityLocked( - p, Activity.RESULT_CANCELED, null, "move-affinity", false); + p, Activity.RESULT_CANCELED, null, null, "move-affinity", false); } } else { if (taskInsertionPoint < 0) { @@ -3535,8 +3536,8 @@ class ActivityStack extends ConfigurationContainer { if (targetNdx > 0) { ActivityRecord p = taskActivities.get(targetNdx - 1); if (p.intent.getComponent().equals(target.intent.getComponent())) { - finishActivityLocked(p, Activity.RESULT_CANCELED, null, "replace", - false); + finishActivityLocked(p, Activity.RESULT_CANCELED, null, null, + "replace", false); } } } @@ -3596,22 +3597,21 @@ class ActivityStack extends ConfigurationContainer { return taskTop; } - void sendActivityResultLocked(int callingUid, ActivityRecord r, - String resultWho, int requestCode, int resultCode, Intent data) { - + void sendActivityResultLocked(int callingUid, ActivityRecord r, String resultWho, + int requestCode, int resultCode, Intent resultData, NeededUriGrants resultGrants) { if (callingUid > 0) { - mService.mUgmInternal.grantUriPermissionFromIntent(callingUid, r.packageName, - data, r.getUriPermissionsLocked(), r.mUserId); + mService.mUgmInternal.grantUriPermissionUncheckedFromIntent(resultGrants, + r.getUriPermissionsLocked()); } if (DEBUG_RESULTS) Slog.v(TAG, "Send activity result to " + r + " : who=" + resultWho + " req=" + requestCode - + " res=" + resultCode + " data=" + data); + + " res=" + resultCode + " data=" + resultData); if (mResumedActivity == r && r.attachedToProcess()) { try { ArrayList<ResultInfo> list = new ArrayList<ResultInfo>(); list.add(new ResultInfo(resultWho, requestCode, - resultCode, data)); + resultCode, resultData)); mService.getLifecycleManager().scheduleTransaction(r.app.getThread(), r.appToken, ActivityResultItem.obtain(list)); return; @@ -3620,7 +3620,7 @@ class ActivityStack extends ConfigurationContainer { } } - r.addResultLocked(null, resultWho, requestCode, resultCode, data); + r.addResultLocked(null, resultWho, requestCode, resultCode, resultData); } /** Returns true if the task is one of the task finishing on-top of the top running task. */ @@ -3727,8 +3727,8 @@ class ActivityStack extends ConfigurationContainer { if (!r.finishing) { if (!shouldSleepActivities()) { if (DEBUG_STATES) Slog.d(TAG_STATES, "no-history finish of " + r); - if (requestFinishActivityLocked(r.appToken, Activity.RESULT_CANCELED, null, - "stop-no-history", false)) { + if (requestFinishActivityLocked(r.appToken, Activity.RESULT_CANCELED, + null, null, "stop-no-history", false)) { // If {@link requestFinishActivityLocked} returns {@code true}, // {@link adjustFocusedActivityStack} would have been already called. r.resumeKeyDispatchingLocked(); @@ -3784,7 +3784,7 @@ class ActivityStack extends ConfigurationContainer { * some reason it is being left as-is. */ final boolean requestFinishActivityLocked(IBinder token, int resultCode, - Intent resultData, String reason, boolean oomAdj) { + Intent resultData, NeededUriGrants resultGrants, String reason, boolean oomAdj) { ActivityRecord r = isInStackLocked(token); if (DEBUG_RESULTS || DEBUG_STATES) Slog.v(TAG_STATES, "Finishing activity token=" + token + " r=" @@ -3794,7 +3794,7 @@ class ActivityStack extends ConfigurationContainer { return false; } - finishActivityLocked(r, resultCode, resultData, reason, oomAdj); + finishActivityLocked(r, resultCode, resultData, resultGrants, reason, oomAdj); return true; } @@ -3806,8 +3806,8 @@ class ActivityStack extends ConfigurationContainer { if (r.resultTo == self && r.requestCode == requestCode) { if ((r.resultWho == null && resultWho == null) || (r.resultWho != null && r.resultWho.equals(resultWho))) { - finishActivityLocked(r, Activity.RESULT_CANCELED, null, "request-sub", - false); + finishActivityLocked(r, Activity.RESULT_CANCELED, null, null, + "request-sub", false); } } } @@ -3837,7 +3837,7 @@ class ActivityStack extends ConfigurationContainer { int activityNdx = task.mActivities.indexOf(r); getDisplay().mDisplayContent.prepareAppTransition( TRANSIT_CRASHING_ACTIVITY_CLOSE, false /* alwaysKeepCurrent */); - finishActivityLocked(r, Activity.RESULT_CANCELED, null, reason, false); + finishActivityLocked(r, Activity.RESULT_CANCELED, null, null, reason, false); finishedTask = task; // Also terminate any activities below it that aren't yet // stopped, to avoid a situation where one will get @@ -3858,7 +3858,7 @@ class ActivityStack extends ConfigurationContainer { if (!r.isActivityTypeHome() || mService.mHomeProcess != r.app) { Slog.w(TAG, " Force finishing activity " + r.intent.getComponent().flattenToShortString()); - finishActivityLocked(r, Activity.RESULT_CANCELED, null, reason, false); + finishActivityLocked(r, Activity.RESULT_CANCELED, null, null, reason, false); } } } @@ -3874,8 +3874,8 @@ class ActivityStack extends ConfigurationContainer { for (int activityNdx = tr.mActivities.size() - 1; activityNdx >= 0; --activityNdx) { ActivityRecord r = tr.mActivities.get(activityNdx); if (!r.finishing) { - finishActivityLocked(r, Activity.RESULT_CANCELED, null, "finish-voice", - false); + finishActivityLocked(r, Activity.RESULT_CANCELED, null, null, + "finish-voice", false); didOne = true; } } @@ -3911,12 +3911,14 @@ class ActivityStack extends ConfigurationContainer { if (!Objects.equals(cur.taskAffinity, r.taskAffinity)) { break; } - finishActivityLocked(cur, Activity.RESULT_CANCELED, null, "request-affinity", true); + finishActivityLocked(cur, Activity.RESULT_CANCELED, null, null, + "request-affinity", true); } return true; } - private void finishActivityResultsLocked(ActivityRecord r, int resultCode, Intent resultData) { + private void finishActivityResultsLocked(ActivityRecord r, int resultCode, Intent resultData, + NeededUriGrants resultGrants) { // send the result ActivityRecord resultTo = r.resultTo; if (resultTo != null) { @@ -3929,9 +3931,8 @@ class ActivityStack extends ConfigurationContainer { } } if (r.info.applicationInfo.uid > 0) { - mService.mUgmInternal.grantUriPermissionFromIntent(r.info.applicationInfo.uid, - resultTo.packageName, resultData, - resultTo.getUriPermissionsLocked(), resultTo.mUserId); + mService.mUgmInternal.grantUriPermissionUncheckedFromIntent(resultGrants, + resultTo.getUriPermissionsLocked()); } resultTo.addResultLocked(r, r.resultWho, r.requestCode, resultCode, resultData); r.resultTo = null; @@ -3947,12 +3948,10 @@ class ActivityStack extends ConfigurationContainer { r.icicle = null; } - /** - * See {@link #finishActivityLocked(ActivityRecord, int, Intent, String, boolean, boolean)} - */ final boolean finishActivityLocked(ActivityRecord r, int resultCode, Intent resultData, - String reason, boolean oomAdj) { - return finishActivityLocked(r, resultCode, resultData, reason, oomAdj, !PAUSE_IMMEDIATELY); + NeededUriGrants resultGrants, String reason, boolean oomAdj) { + return finishActivityLocked(r, resultCode, resultData, resultGrants, reason, oomAdj, + !PAUSE_IMMEDIATELY); } /** @@ -3960,7 +3959,7 @@ class ActivityStack extends ConfigurationContainer { * list, or false if it is still in the list and will be removed later. */ final boolean finishActivityLocked(ActivityRecord r, int resultCode, Intent resultData, - String reason, boolean oomAdj, boolean pauseImmediately) { + NeededUriGrants resultGrants, String reason, boolean oomAdj, boolean pauseImmediately) { if (r.finishing) { Slog.w(TAG, "Duplicate finish request for " + r); return false; @@ -3990,7 +3989,7 @@ class ActivityStack extends ConfigurationContainer { adjustFocusedActivityStack(r, "finishActivity"); - finishActivityResultsLocked(r, resultCode, resultData); + finishActivityResultsLocked(r, resultCode, resultData, resultGrants); final boolean endTask = index <= 0 && !task.isClearingToReuseTask(); final int transit = endTask ? TRANSIT_TASK_CLOSE : TRANSIT_ACTIVITY_CLOSE; @@ -4223,8 +4222,9 @@ class ActivityStack extends ConfigurationContainer { return false; } - final boolean navigateUpToLocked(ActivityRecord srec, Intent destIntent, int resultCode, - Intent resultData) { + final boolean navigateUpToLocked(ActivityRecord srec, Intent destIntent, + NeededUriGrants destGrants, int resultCode, Intent resultData, + NeededUriGrants resultGrants) { final TaskRecord task = srec.getTaskRecord(); final ArrayList<ActivityRecord> activities = task.mActivities; final int start = activities.indexOf(srec); @@ -4271,7 +4271,8 @@ class ActivityStack extends ConfigurationContainer { final long origId = Binder.clearCallingIdentity(); for (int i = start; i > finishTo; i--) { ActivityRecord r = activities.get(i); - requestFinishActivityLocked(r.appToken, resultCode, resultData, "navigate-up", true); + requestFinishActivityLocked(r.appToken, resultCode, resultData, resultGrants, + "navigate-up", true); // Only return the supplied result for the first activity finished resultCode = Activity.RESULT_CANCELED; resultData = null; @@ -4285,7 +4286,7 @@ class ActivityStack extends ConfigurationContainer { parentLaunchMode == ActivityInfo.LAUNCH_SINGLE_TOP || (destIntentFlags & Intent.FLAG_ACTIVITY_CLEAR_TOP) != 0) { parent.deliverNewIntentLocked(srec.info.applicationInfo.uid, destIntent, - srec.packageName); + destGrants, srec.packageName); } else { try { ActivityInfo aInfo = AppGlobals.getPackageManager().getActivityInfo( @@ -4309,7 +4310,7 @@ class ActivityStack extends ConfigurationContainer { foundParentInTask = false; } requestFinishActivityLocked(parent.appToken, resultCode, - resultData, "navigate-top", true); + resultData, resultGrants, "navigate-top", true); } } Binder.restoreCallingIdentity(origId); @@ -4394,7 +4395,7 @@ class ActivityStack extends ConfigurationContainer { } private void removeActivityFromHistoryLocked(ActivityRecord r, String reason) { - finishActivityResultsLocked(r, Activity.RESULT_CANCELED, null); + finishActivityResultsLocked(r, Activity.RESULT_CANCELED, null, null); r.makeFinishingLocked(); if (DEBUG_ADD_REMOVE) Slog.i(TAG_ADD_REMOVE, "Removing activity " + r + " from stack callers=" + Debug.getCallers(5)); @@ -5126,7 +5127,8 @@ class ActivityStack extends ConfigurationContainer { for (int activityNdx = activities.size() - 1; activityNdx >= 0; --activityNdx) { final ActivityRecord r = activities.get(activityNdx); if ((r.info.flags&ActivityInfo.FLAG_FINISH_ON_CLOSE_SYSTEM_DIALOGS) != 0) { - finishActivityLocked(r, Activity.RESULT_CANCELED, null, "close-sys", true); + finishActivityLocked(r, Activity.RESULT_CANCELED, null, null, + "close-sys", true); } } } @@ -5170,8 +5172,8 @@ class ActivityStack extends ConfigurationContainer { didSomething = true; Slog.i(TAG, " Force finishing activity " + r); lastTask = r.getTaskRecord(); - finishActivityLocked(r, Activity.RESULT_CANCELED, null, "force-stop", - true); + finishActivityLocked(r, Activity.RESULT_CANCELED, null, null, + "force-stop", true); } } } @@ -5225,8 +5227,8 @@ class ActivityStack extends ConfigurationContainer { final ArrayList<ActivityRecord> activities = mTaskHistory.get(top).mActivities; int activityTop = activities.size() - 1; if (activityTop >= 0) { - finishActivityLocked(activities.get(activityTop), Activity.RESULT_CANCELED, null, - "unhandled-back", true); + finishActivityLocked(activities.get(activityTop), Activity.RESULT_CANCELED, + null, null, "unhandled-back", true); } } } diff --git a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java index c992a69c2ecb..8d2a92acc91f 100644 --- a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java +++ b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java @@ -140,6 +140,7 @@ import com.android.internal.util.function.pooled.PooledLambda; import com.android.server.am.ActivityManagerService; import com.android.server.am.EventLogTags; import com.android.server.am.UserState; +import com.android.server.uri.NeededUriGrants; import java.io.FileDescriptor; import java.io.IOException; @@ -403,14 +404,17 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks { final int startFlags; final ActivityStack stack; final WindowProcessController callerApp; + final NeededUriGrants neededGrants; - PendingActivityLaunch(ActivityRecord _r, ActivityRecord _sourceRecord, - int _startFlags, ActivityStack _stack, WindowProcessController app) { - r = _r; - sourceRecord = _sourceRecord; - startFlags = _startFlags; - stack = _stack; - callerApp = app; + PendingActivityLaunch(ActivityRecord r, ActivityRecord sourceRecord, + int startFlags, ActivityStack stack, WindowProcessController callerApp, + NeededUriGrants neededGrants) { + this.r = r; + this.sourceRecord = sourceRecord; + this.startFlags = startFlags; + this.stack = stack; + this.callerApp = callerApp; + this.neededGrants = neededGrants; } void sendErrorResult(String message) { @@ -874,8 +878,8 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks { Slog.e(TAG, "Second failure launching " + r.intent.getComponent().flattenToShortString() + ", giving up", e); proc.appDied(); - stack.requestFinishActivityLocked(r.appToken, Activity.RESULT_CANCELED, null, - "2nd-crash", false); + stack.requestFinishActivityLocked(r.appToken, Activity.RESULT_CANCELED, + null, null, "2nd-crash", false); return false; } @@ -1020,7 +1024,7 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks { if (resultRecord != null) { resultStack.sendActivityResultLocked(-1, resultRecord, resultWho, requestCode, - Activity.RESULT_CANCELED, null); + Activity.RESULT_CANCELED, null, null); } final String msg; if (actionRestriction == ACTIVITY_RESTRICTION_PERMISSION) { diff --git a/services/core/java/com/android/server/wm/ActivityStartController.java b/services/core/java/com/android/server/wm/ActivityStartController.java index 919141c13622..738d143280d9 100644 --- a/services/core/java/com/android/server/wm/ActivityStartController.java +++ b/services/core/java/com/android/server/wm/ActivityStartController.java @@ -462,7 +462,7 @@ public class ActivityStartController { "pendingActivityLaunch"); try { starter.startResolvedActivity(pal.r, pal.sourceRecord, null, null, pal.startFlags, - resume, pal.r.pendingOptions, null); + resume, pal.r.pendingOptions, null, pal.neededGrants); } catch (Exception e) { Slog.e(TAG, "Exception during pending activity launch pal=" + pal, e); pal.sendErrorResult(e.getMessage()); diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java index 1718ac634b10..fb18569ba637 100644 --- a/services/core/java/com/android/server/wm/ActivityStarter.java +++ b/services/core/java/com/android/server/wm/ActivityStarter.java @@ -127,6 +127,7 @@ import com.android.internal.app.IVoiceInteractor; import com.android.server.am.EventLogTags; import com.android.server.am.PendingIntentRecord; import com.android.server.pm.InstantAppResolver; +import com.android.server.uri.NeededUriGrants; import com.android.server.wm.ActivityStackSupervisor.PendingActivityLaunch; import com.android.server.wm.LaunchParamsController.LaunchParams; @@ -549,7 +550,8 @@ class ActivityStarter { */ int startResolvedActivity(final ActivityRecord r, ActivityRecord sourceRecord, IVoiceInteractionSession voiceSession, IVoiceInteractor voiceInteractor, - int startFlags, boolean doResume, ActivityOptions options, TaskRecord inTask) { + int startFlags, boolean doResume, ActivityOptions options, TaskRecord inTask, + NeededUriGrants neededGrants) { try { mSupervisor.getActivityMetricsLogger().notifyActivityLaunching(r.intent); mLastStartReason = "startResolvedActivity"; @@ -557,7 +559,7 @@ class ActivityStarter { mLastStartActivityRecord[0] = r; mLastStartActivityResult = startActivity(r, sourceRecord, voiceSession, voiceInteractor, startFlags, doResume, options, inTask, mLastStartActivityRecord, - false /* restrictedBgActivity */); + false /* restrictedBgActivity */, neededGrants); mSupervisor.getActivityMetricsLogger().notifyActivityLaunched(mLastStartActivityResult, mLastStartActivityRecord[0]); return mLastStartActivityResult; @@ -576,6 +578,33 @@ class ActivityStarter { boolean allowPendingRemoteAnimationRegistryLookup, PendingIntentRecord originatingPendingIntent, boolean allowBackgroundActivityStart) { + // Carefully collect grants without holding lock + NeededUriGrants neededGrants = null; + if (aInfo != null) { + neededGrants = mService.mUgmInternal.checkGrantUriPermissionFromIntent( + resolveCallingUid(mRequest.caller), intent, aInfo.applicationInfo.packageName, + UserHandle.getUserId(aInfo.applicationInfo.uid)); + } + + return startActivity(caller, intent, ephemeralIntent, resolvedType, aInfo, rInfo, + voiceSession, voiceInteractor, resultTo, resultWho, requestCode, callingPid, + callingUid, callingPackage, realCallingPid, realCallingUid, startFlags, options, + ignoreTargetSecurity, componentSpecified, outActivity, inTask, reason, + allowPendingRemoteAnimationRegistryLookup, originatingPendingIntent, + allowBackgroundActivityStart, neededGrants); + } + + private int startActivity(IApplicationThread caller, Intent intent, Intent ephemeralIntent, + String resolvedType, ActivityInfo aInfo, ResolveInfo rInfo, + IVoiceInteractionSession voiceSession, IVoiceInteractor voiceInteractor, + IBinder resultTo, String resultWho, int requestCode, int callingPid, int callingUid, + String callingPackage, int realCallingPid, int realCallingUid, int startFlags, + SafeActivityOptions options, boolean ignoreTargetSecurity, boolean componentSpecified, + ActivityRecord[] outActivity, TaskRecord inTask, String reason, + boolean allowPendingRemoteAnimationRegistryLookup, + PendingIntentRecord originatingPendingIntent, boolean allowBackgroundActivityStart, + NeededUriGrants neededGrants) { + if (TextUtils.isEmpty(reason)) { throw new IllegalArgumentException("Need to specify a reason."); } @@ -588,7 +617,7 @@ class ActivityStarter { callingPid, callingUid, callingPackage, realCallingPid, realCallingUid, startFlags, options, ignoreTargetSecurity, componentSpecified, mLastStartActivityRecord, inTask, allowPendingRemoteAnimationRegistryLookup, originatingPendingIntent, - allowBackgroundActivityStart); + allowBackgroundActivityStart, neededGrants); if (outActivity != null) { // mLastStartActivityRecord[0] is set in the call to startActivity above. @@ -619,7 +648,8 @@ class ActivityStarter { SafeActivityOptions options, boolean ignoreTargetSecurity, boolean componentSpecified, ActivityRecord[] outActivity, TaskRecord inTask, boolean allowPendingRemoteAnimationRegistryLookup, - PendingIntentRecord originatingPendingIntent, boolean allowBackgroundActivityStart) { + PendingIntentRecord originatingPendingIntent, boolean allowBackgroundActivityStart, + NeededUriGrants neededGrants) { mSupervisor.getActivityMetricsLogger().notifyActivityLaunching(intent); int err = ActivityManager.START_SUCCESS; // Pull the optional Ephemeral Installer-only bundle out of the options early. @@ -754,7 +784,7 @@ class ActivityStarter { if (err != START_SUCCESS) { if (resultRecord != null) { resultStack.sendActivityResultLocked( - -1, resultRecord, resultWho, requestCode, RESULT_CANCELED, null); + -1, resultRecord, resultWho, requestCode, RESULT_CANCELED, null, null); } SafeActivityOptions.abort(options); return err; @@ -814,12 +844,16 @@ class ActivityStarter { callingPid = mInterceptor.mCallingPid; callingUid = mInterceptor.mCallingUid; checkedOptions = mInterceptor.mActivityOptions; + + // The interception target shouldn't get any permission grants + // intended for the original destination + neededGrants = null; } if (abort) { if (resultRecord != null) { resultStack.sendActivityResultLocked(-1, resultRecord, resultWho, requestCode, - RESULT_CANCELED, null); + RESULT_CANCELED, null, null); } // We pretend to the caller that it was really started, but // they will just get a cancel result. @@ -875,6 +909,10 @@ class ActivityStarter { aInfo = mSupervisor.resolveActivity(intent, rInfo, startFlags, null /*profilerInfo*/); + // The permissions review target shouldn't get any permission + // grants intended for the original destination + neededGrants = null; + if (DEBUG_PERMISSIONS_REVIEW) { final ActivityStack focusedStack = mRootActivityContainer.getTopDisplayFocusedStack(); @@ -897,6 +935,10 @@ class ActivityStarter { callingPid = realCallingPid; aInfo = mSupervisor.resolveActivity(intent, rInfo, startFlags, null /*profilerInfo*/); + + // The ephemeral installer shouldn't get any permission grants + // intended for the original destination + neededGrants = null; } ActivityRecord r = new ActivityRecord(mService, callerApp, callingPid, callingUid, @@ -923,7 +965,7 @@ class ActivityStarter { realCallingPid, realCallingUid, "Activity start")) { if (!(restrictedBgActivity && handleBackgroundActivityAbort(r))) { mController.addPendingActivityLaunch(new PendingActivityLaunch(r, - sourceRecord, startFlags, stack, callerApp)); + sourceRecord, startFlags, stack, callerApp, neededGrants)); } ActivityOptions.abort(checkedOptions); return ActivityManager.START_SWITCHES_CANCELED; @@ -934,7 +976,8 @@ class ActivityStarter { mController.doPendingActivityLaunches(false); final int res = startActivity(r, sourceRecord, voiceSession, voiceInteractor, startFlags, - true /* doResume */, checkedOptions, inTask, outActivity, restrictedBgActivity); + true /* doResume */, checkedOptions, inTask, outActivity, restrictedBgActivity, + neededGrants); mSupervisor.getActivityMetricsLogger().notifyActivityLaunched(res, outActivity[0]); return res; } @@ -1232,9 +1275,15 @@ class ActivityStarter { } } } + // Collect information about the target of the Intent. ActivityInfo aInfo = mSupervisor.resolveActivity(intent, rInfo, startFlags, profilerInfo); + // Carefully collect grants without holding lock + NeededUriGrants neededGrants = mService.mUgmInternal.checkGrantUriPermissionFromIntent( + resolveCallingUid(mRequest.caller), intent, aInfo.applicationInfo.packageName, + UserHandle.getUserId(aInfo.applicationInfo.uid)); + synchronized (mService.mGlobalLock) { final ActivityStack stack = mRootActivityContainer.getTopDisplayFocusedStack(); stack.mConfigWillChange = globalConfig != null @@ -1311,7 +1360,7 @@ class ActivityStarter { callingUid, callingPackage, realCallingPid, realCallingUid, startFlags, options, ignoreTargetSecurity, componentSpecified, outRecord, inTask, reason, allowPendingRemoteAnimationRegistryLookup, originatingPendingIntent, - allowBackgroundActivityStart); + allowBackgroundActivityStart, neededGrants); Binder.restoreCallingIdentity(origId); @@ -1406,14 +1455,16 @@ class ActivityStarter { private int startActivity(final ActivityRecord r, ActivityRecord sourceRecord, IVoiceInteractionSession voiceSession, IVoiceInteractor voiceInteractor, - int startFlags, boolean doResume, ActivityOptions options, TaskRecord inTask, - ActivityRecord[] outActivity, boolean restrictedBgActivity) { + int startFlags, boolean doResume, ActivityOptions options, TaskRecord inTask, + ActivityRecord[] outActivity, boolean restrictedBgActivity, + NeededUriGrants neededGrants) { int result = START_CANCELED; final ActivityStack startedActivityStack; try { mService.mWindowManager.deferSurfaceLayout(); result = startActivityUnchecked(r, sourceRecord, voiceSession, voiceInteractor, - startFlags, doResume, options, inTask, outActivity, restrictedBgActivity); + startFlags, doResume, options, inTask, outActivity, restrictedBgActivity, + neededGrants); } finally { final ActivityStack currentStack = r.getActivityStack(); startedActivityStack = currentStack != null ? currentStack : mTargetStack; @@ -1438,7 +1489,8 @@ class ActivityStarter { final ActivityStack stack = mStartActivity.getActivityStack(); if (stack != null) { stack.finishActivityLocked(mStartActivity, RESULT_CANCELED, - null /* intentResultData */, "startActivity", true /* oomAdj */); + null /* resultData */, null /* resultGrants */, + "startActivity", true /* oomAdj */); } } mService.mWindowManager.continueSurfaceLayout(); @@ -1467,7 +1519,7 @@ class ActivityStarter { if (resultRecord != null) { ActivityStack resultStack = resultRecord.getActivityStack(); resultStack.sendActivityResultLocked(-1, resultRecord, resultWho, requestCode, - RESULT_CANCELED, null); + RESULT_CANCELED, null, null); } // We pretend to the caller that it was really started to make it backward compatible, but // they will just get a cancel result. @@ -1479,7 +1531,8 @@ class ActivityStarter { private int startActivityUnchecked(final ActivityRecord r, ActivityRecord sourceRecord, IVoiceInteractionSession voiceSession, IVoiceInteractor voiceInteractor, int startFlags, boolean doResume, ActivityOptions options, TaskRecord inTask, - ActivityRecord[] outActivity, boolean restrictedBgActivity) { + ActivityRecord[] outActivity, boolean restrictedBgActivity, + NeededUriGrants neededGrants) { setInitialState(r, options, inTask, doResume, startFlags, sourceRecord, voiceSession, voiceInteractor, restrictedBgActivity); @@ -1629,7 +1682,7 @@ class ActivityStarter { if (sourceStack != null) { sourceStack.sendActivityResultLocked(-1 /* callingUid */, mStartActivity.resultTo, mStartActivity.resultWho, mStartActivity.requestCode, RESULT_CANCELED, - null /* data */); + null /* resultData */, null /* resultGrants */); } ActivityOptions.abort(mOptions); return START_CLASS_NOT_FOUND; @@ -1696,8 +1749,8 @@ class ActivityStarter { return result; } - mService.mUgmInternal.grantUriPermissionFromIntent(mCallingUid, mStartActivity.packageName, - mIntent, mStartActivity.getUriPermissionsLocked(), mStartActivity.mUserId); + mService.mUgmInternal.grantUriPermissionUncheckedFromIntent(neededGrants, + mStartActivity.getUriPermissionsLocked()); mService.getPackageManagerInternalLocked().grantEphemeralAccess( mStartActivity.mUserId, mIntent, UserHandle.getAppId(mStartActivity.appInfo.uid), UserHandle.getAppId(mCallingUid)); @@ -1933,7 +1986,7 @@ class ActivityStarter { Slog.w(TAG, "Activity is launching as a new task, so cancelling activity result."); sourceStack.sendActivityResultLocked(-1 /* callingUid */, mStartActivity.resultTo, mStartActivity.resultWho, mStartActivity.requestCode, RESULT_CANCELED, - null /* data */); + null /* resultData */, null /* resultGrants */); mStartActivity.resultTo = null; } } @@ -2362,8 +2415,13 @@ class ActivityStarter { } ActivityStack.logStartActivity(AM_NEW_INTENT, activity, activity.getTaskRecord()); - activity.deliverNewIntentLocked(mCallingUid, mStartActivity.intent, + + Intent intent = mStartActivity.intent; + NeededUriGrants intentGrants = mService.mUgmInternal.checkGrantUriPermissionFromIntent( + mCallingUid, intent, activity.packageName, activity.mUserId); + activity.deliverNewIntentLocked(mCallingUid, intent, intentGrants, mStartActivity.launchedFromPackage); + mIntentDelivered = true; } @@ -2742,6 +2800,18 @@ class ActivityStarter { } } + private int resolveCallingUid(IApplicationThread caller) { + if (caller != null) { + synchronized (mService.mGlobalLock) { + final WindowProcessController callerApp = mService.getProcessController(caller); + if (callerApp != null) { + return callerApp.mInfo.uid; + } + } + } + return -1; + } + private boolean isLaunchModeOneOf(int mode1, int mode2) { return mode1 == mLaunchMode || mode2 == mLaunchMode; } diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index 765c9d0a5864..f8bfacbefc2c 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -265,6 +265,7 @@ import com.android.server.appop.AppOpsService; import com.android.server.firewall.IntentFirewall; import com.android.server.pm.UserManagerService; import com.android.server.policy.PermissionPolicyInternal; +import com.android.server.uri.NeededUriGrants; import com.android.server.uri.UriGrantsManagerInternal; import com.android.server.vr.VrManagerInternal; @@ -1540,11 +1541,19 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { throw new IllegalArgumentException("File descriptors passed in Intent"); } + final ActivityRecord r; synchronized (mGlobalLock) { - ActivityRecord r = ActivityRecord.isInStackLocked(token); + r = ActivityRecord.isInStackLocked(token); if (r == null) { return true; } + } + + // Carefully collect grants without holding lock + final NeededUriGrants resultGrants = mUgmInternal.checkGrantUriPermissionFromIntent( + Binder.getCallingUid(), resultData, r.packageName, r.mUserId); + + synchronized (mGlobalLock) { // Keep track of the root activity of the task before we finish it final TaskRecord tr = r.getTaskRecord(); ActivityRecord rootR = tr.getRootActivity(); @@ -1606,7 +1615,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { r.mRelaunchReason = RELAUNCH_REASON_NONE; } else { res = tr.getStack().requestFinishActivityLocked(token, resultCode, - resultData, "app-request", true); + resultData, resultGrants, "app-request", true); if (!res) { Slog.i(TAG, "Failed to finish by app-request"); } @@ -2132,14 +2141,23 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { @Override public boolean navigateUpTo(IBinder token, Intent destIntent, int resultCode, Intent resultData) { - + final ActivityRecord r; synchronized (mGlobalLock) { - final ActivityRecord r = ActivityRecord.forTokenLocked(token); - if (r != null) { - return r.getActivityStack().navigateUpToLocked( - r, destIntent, resultCode, resultData); + r = ActivityRecord.isInStackLocked(token); + if (r == null) { + return false; } - return false; + } + + // Carefully collect grants without holding lock + final NeededUriGrants destGrants = mUgmInternal.checkGrantUriPermissionFromIntent( + Binder.getCallingUid(), destIntent, r.packageName, r.mUserId); + final NeededUriGrants resultGrants = mUgmInternal.checkGrantUriPermissionFromIntent( + Binder.getCallingUid(), resultData, r.packageName, r.mUserId); + + synchronized (mGlobalLock) { + return r.getActivityStack().navigateUpToLocked( + r, destIntent, destGrants, resultCode, resultData, resultGrants); } } @@ -6589,14 +6607,23 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { @Override public void sendActivityResult(int callingUid, IBinder activityToken, String resultWho, - int requestCode, int resultCode, Intent data) { + int requestCode, int resultCode, Intent resultData) { + final ActivityRecord r; synchronized (mGlobalLock) { - final ActivityRecord r = ActivityRecord.isInStackLocked(activityToken); - if (r != null && r.getActivityStack() != null) { - r.getActivityStack().sendActivityResultLocked(callingUid, r, resultWho, - requestCode, resultCode, data); + r = ActivityRecord.isInStackLocked(activityToken); + if (r == null || r.getActivityStack() == null) { + return; } } + + // Carefully collect grants without holding lock + final NeededUriGrants resultGrants = mUgmInternal.checkGrantUriPermissionFromIntent( + Binder.getCallingUid(), resultData, r.packageName, r.mUserId); + + synchronized (mGlobalLock) { + r.getActivityStack().sendActivityResultLocked(callingUid, r, resultWho, + requestCode, resultCode, resultData, resultGrants); + } } @Override diff --git a/services/core/java/com/android/server/wm/TaskRecord.java b/services/core/java/com/android/server/wm/TaskRecord.java index 298b302a17fb..4a0bf02ab517 100644 --- a/services/core/java/com/android/server/wm/TaskRecord.java +++ b/services/core/java/com/android/server/wm/TaskRecord.java @@ -1442,7 +1442,7 @@ class TaskRecord extends ConfigurationContainer { mActivities.remove(activityNdx); --activityNdx; --numActivities; - } else if (mStack.finishActivityLocked(r, Activity.RESULT_CANCELED, null, + } else if (mStack.finishActivityLocked(r, Activity.RESULT_CANCELED, null, null, reason, false, pauseImmediately)) { --activityNdx; --numActivities; @@ -1497,8 +1497,8 @@ class TaskRecord extends ConfigurationContainer { if (opts != null) { ret.updateOptionsLocked(opts); } - if (mStack != null && mStack.finishActivityLocked( - r, Activity.RESULT_CANCELED, null, "clear-task-stack", false)) { + if (mStack != null && mStack.finishActivityLocked(r, Activity.RESULT_CANCELED, + null, null, "clear-task-stack", false)) { --activityNdx; --numActivities; } @@ -1512,8 +1512,8 @@ class TaskRecord extends ConfigurationContainer { && !ActivityStarter.isDocumentLaunchesIntoExisting(launchFlags)) { if (!ret.finishing) { if (mStack != null) { - mStack.finishActivityLocked( - ret, Activity.RESULT_CANCELED, null, "clear-task-top", false); + mStack.finishActivityLocked(ret, Activity.RESULT_CANCELED, + null, null, "clear-task-top", false); } return null; } diff --git a/services/core/java/com/android/server/wm/WindowProcessController.java b/services/core/java/com/android/server/wm/WindowProcessController.java index 2ad25cf123e2..fbd4ec7ff536 100644 --- a/services/core/java/com/android/server/wm/WindowProcessController.java +++ b/services/core/java/com/android/server/wm/WindowProcessController.java @@ -623,7 +623,7 @@ public class WindowProcessController extends ConfigurationContainer<Configuratio final ActivityRecord r = activities.get(i); if (!r.finishing && r.isInStackLocked()) { r.getActivityStack().finishActivityLocked(r, Activity.RESULT_CANCELED, - null, "finish-heavy", true); + null, null, "finish-heavy", true); } } } diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java index 757267e56fa2..4c0b582e8860 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java @@ -940,7 +940,8 @@ public class ActivityStackTests extends ActivityTestsBase { homeStask.removeTask(homeTask, "testAdjustFocusedStack", REMOVE_TASK_MODE_DESTROYING); // Finish the only activity. - mStack.finishActivityLocked(topActivity, 0 /* resultCode */, null /* resultData */, + mStack.finishActivityLocked(topActivity, 0 /* resultCode */, + null /* resultData */, null /* resultGrants */, "testAdjustFocusedStack", false /* oomAdj */); // Although home stack is empty, it should still be the focused stack. assertEquals(homeStask, mDefaultDisplay.getFocusedStack()); diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityStartControllerTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityStartControllerTests.java index a7bbe6e4cf02..2d1200675fd4 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityStartControllerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityStartControllerTests.java @@ -82,12 +82,12 @@ public class ActivityStartControllerTests extends ActivityTestsBase { wpc.setThread(mock(IApplicationThread.class)); mController.addPendingActivityLaunch( - new PendingActivityLaunch(activity, source, startFlags, stack, wpc)); + new PendingActivityLaunch(activity, source, startFlags, stack, wpc, null)); final boolean resume = random.nextBoolean(); mController.doPendingActivityLaunches(resume); verify(mStarter, times(1)).startResolvedActivity(eq(activity), eq(source), eq(null), - eq(null), eq(startFlags), eq(resume), eq(null), eq(null)); + eq(null), eq(startFlags), eq(resume), eq(null), eq(null), eq(null)); } |