From 29b2516012cf69022ebe33409223095fb3c99405 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Fri, 19 Sep 2014 09:14:34 -0700
Subject: [PATCH] Only revoke ownerless grants when unprivileged.

Recently we relaxed revokeUriPermission() to allow apps to revoke
Uri permissions that had been granted to them, but this uncovered
bugs in apps that had been relying on the previous no-op behavior.

To mitigate this, only revoke ownerless Uri permissions when in the
unprivileged state; an active owner indicates that another component
of the calling app still needs the permission.

Bug: 17554268
Change-Id: Icc412933b29041ffb699d20136a623440ecc71ec
---
 .../server/am/ActivityManagerService.java        | 16 ++++++++--------
 .../com/android/server/am/UriPermission.java     |  6 +++---
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 741cffe8685ad..190c16c06740b 100755
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -7528,8 +7528,8 @@ public final class ActivityManagerService extends ActivityManagerNative
 
         // Does the caller have this permission on the URI?
         if (!checkHoldingPermissionsLocked(pm, pi, grantUri, callingUid, modeFlags)) {
-            // Have they don't have direct access to the URI, then revoke any URI
-            // permissions that have been granted to them.
+            // If they don't have direct access to the URI, then revoke any
+            // ownerless URI permissions that have been granted to them.
             final ArrayMap<GrantUri, UriPermission> perms = mGrantedUriPermissions.get(callingUid);
             if (perms != null) {
                 boolean persistChanged = false;
@@ -7538,10 +7538,10 @@ public final class ActivityManagerService extends ActivityManagerNative
                     if (perm.uri.sourceUserId == grantUri.sourceUserId
                             && perm.uri.uri.isPathPrefixMatch(grantUri.uri)) {
                         if (DEBUG_URI_PERMISSION)
-                            Slog.v(TAG,
-                                    "Revoking " + perm.targetUid + " permission to " + perm.uri);
+                            Slog.v(TAG, "Revoking non-owned " + perm.targetUid +
+                                    " permission to " + perm.uri);
                         persistChanged |= perm.revokeModes(
-                                modeFlags | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION);
+                                modeFlags | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION, false);
                         if (perm.modeFlags == 0) {
                             it.remove();
                         }
@@ -7573,7 +7573,7 @@ public final class ActivityManagerService extends ActivityManagerNative
                         Slog.v(TAG,
                                 "Revoking " + perm.targetUid + " permission to " + perm.uri);
                     persistChanged |= perm.revokeModes(
-                            modeFlags | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION);
+                            modeFlags | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION, true);
                     if (perm.modeFlags == 0) {
                         it.remove();
                     }
@@ -7661,8 +7661,8 @@ public final class ActivityManagerService extends ActivityManagerNative
                     // Only inspect grants matching package
                     if (packageName == null || perm.sourcePkg.equals(packageName)
                             || perm.targetPkg.equals(packageName)) {
-                        persistChanged |= perm.revokeModes(
-                                persistable ? ~0 : ~Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION);
+                        persistChanged |= perm.revokeModes(persistable
+                                ? ~0 : ~Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION, true);
 
                         // Only remove when no modes remain; any persisted grants
                         // will keep this alive.
diff --git a/services/core/java/com/android/server/am/UriPermission.java b/services/core/java/com/android/server/am/UriPermission.java
index 284086dcbd799..91daf776a061c 100644
--- a/services/core/java/com/android/server/am/UriPermission.java
+++ b/services/core/java/com/android/server/am/UriPermission.java
@@ -180,7 +180,7 @@ final class UriPermission {
     /**
      * @return if mode changes should trigger persisting.
      */
-    boolean revokeModes(int modeFlags) {
+    boolean revokeModes(int modeFlags, boolean includingOwners) {
         final boolean persistable = (modeFlags & Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION) != 0;
         modeFlags &= (Intent.FLAG_GRANT_READ_URI_PERMISSION
                 | Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
@@ -193,7 +193,7 @@ final class UriPermission {
                 persistedModeFlags &= ~Intent.FLAG_GRANT_READ_URI_PERMISSION;
             }
             globalModeFlags &= ~Intent.FLAG_GRANT_READ_URI_PERMISSION;
-            if (mReadOwners != null) {
+            if (mReadOwners != null && includingOwners) {
                 ownedModeFlags &= ~Intent.FLAG_GRANT_READ_URI_PERMISSION;
                 for (UriPermissionOwner r : mReadOwners) {
                     r.removeReadPermission(this);
@@ -207,7 +207,7 @@ final class UriPermission {
                 persistedModeFlags &= ~Intent.FLAG_GRANT_WRITE_URI_PERMISSION;
             }
             globalModeFlags &= ~Intent.FLAG_GRANT_WRITE_URI_PERMISSION;
-            if (mWriteOwners != null) {
+            if (mWriteOwners != null && includingOwners) {
                 ownedModeFlags &= ~Intent.FLAG_GRANT_WRITE_URI_PERMISSION;
                 for (UriPermissionOwner r : mWriteOwners) {
                     r.removeWritePermission(this);