commit 344ce7ce703f806f50ea253047d6dbbb10eaa424
author Jeff Sharkey <jsharkey@android.com> Wed May 29 14:52:59 2019 -0600
committer Jeff Sharkey <jsharkey@android.com> Thu May 30 11:29:49 2019 -0600
tree 6bd74ba11afcc31dcafe1b83f0118f953e191e34
parent 190e99cf3ab47b2eb196999aeab92a9eee98cae5

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

services/core/java/com/android/server/wm/ActivityTaskManagerService.java

diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
index 765c9d0..f8bfacb 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.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 @@
             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 @@
                     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 @@
     @Override
     public boolean navigateUpTo(IBinder token, Intent destIntent, int resultCode,
             Intent resultData) {
+        final ActivityRecord r;
+        synchronized (mGlobalLock) {
+            r = ActivityRecord.isInStackLocked(token);
+            if (r == null) {
+                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) {
-            final ActivityRecord r = ActivityRecord.forTokenLocked(token);
-            if (r != null) {
-                return r.getActivityStack().navigateUpToLocked(
-                        r, destIntent, resultCode, resultData);
-            }
-            return false;
+            return r.getActivityStack().navigateUpToLocked(
+                    r, destIntent, destGrants, resultCode, resultData, resultGrants);
         }
     }
 
@@ -6589,14 +6607,23 @@
 
         @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