Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 29b25160 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

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
parent 8f2152c0
Loading
Loading
Loading
Loading
+8 −8
Original line number Original line Diff line number Diff line
@@ -7528,8 +7528,8 @@ public final class ActivityManagerService extends ActivityManagerNative
        // Does the caller have this permission on the URI?
        // Does the caller have this permission on the URI?
        if (!checkHoldingPermissionsLocked(pm, pi, grantUri, callingUid, modeFlags)) {
        if (!checkHoldingPermissionsLocked(pm, pi, grantUri, callingUid, modeFlags)) {
            // Have they don't have direct access to the URI, then revoke any URI
            // If they don't have direct access to the URI, then revoke any
            // permissions that have been granted to them.
            // ownerless URI permissions that have been granted to them.
            final ArrayMap<GrantUri, UriPermission> perms = mGrantedUriPermissions.get(callingUid);
            final ArrayMap<GrantUri, UriPermission> perms = mGrantedUriPermissions.get(callingUid);
            if (perms != null) {
            if (perms != null) {
                boolean persistChanged = false;
                boolean persistChanged = false;
@@ -7538,10 +7538,10 @@ public final class ActivityManagerService extends ActivityManagerNative
                    if (perm.uri.sourceUserId == grantUri.sourceUserId
                    if (perm.uri.sourceUserId == grantUri.sourceUserId
                            && perm.uri.uri.isPathPrefixMatch(grantUri.uri)) {
                            && perm.uri.uri.isPathPrefixMatch(grantUri.uri)) {
                        if (DEBUG_URI_PERMISSION)
                        if (DEBUG_URI_PERMISSION)
                            Slog.v(TAG,
                            Slog.v(TAG, "Revoking non-owned " + perm.targetUid +
                                    "Revoking " + perm.targetUid + " permission to " + perm.uri);
                                    " permission to " + perm.uri);
                        persistChanged |= perm.revokeModes(
                        persistChanged |= perm.revokeModes(
                                modeFlags | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION);
                                modeFlags | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION, false);
                        if (perm.modeFlags == 0) {
                        if (perm.modeFlags == 0) {
                            it.remove();
                            it.remove();
                        }
                        }
@@ -7573,7 +7573,7 @@ public final class ActivityManagerService extends ActivityManagerNative
                        Slog.v(TAG,
                        Slog.v(TAG,
                                "Revoking " + perm.targetUid + " permission to " + perm.uri);
                                "Revoking " + perm.targetUid + " permission to " + perm.uri);
                    persistChanged |= perm.revokeModes(
                    persistChanged |= perm.revokeModes(
                            modeFlags | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION);
                            modeFlags | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION, true);
                    if (perm.modeFlags == 0) {
                    if (perm.modeFlags == 0) {
                        it.remove();
                        it.remove();
                    }
                    }
@@ -7661,8 +7661,8 @@ public final class ActivityManagerService extends ActivityManagerNative
                    // Only inspect grants matching package
                    // Only inspect grants matching package
                    if (packageName == null || perm.sourcePkg.equals(packageName)
                    if (packageName == null || perm.sourcePkg.equals(packageName)
                            || perm.targetPkg.equals(packageName)) {
                            || perm.targetPkg.equals(packageName)) {
                        persistChanged |= perm.revokeModes(
                        persistChanged |= perm.revokeModes(persistable
                                persistable ? ~0 : ~Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION);
                                ? ~0 : ~Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION, true);
                        // Only remove when no modes remain; any persisted grants
                        // Only remove when no modes remain; any persisted grants
                        // will keep this alive.
                        // will keep this alive.
+3 −3
Original line number Original line Diff line number Diff line
@@ -180,7 +180,7 @@ final class UriPermission {
    /**
    /**
     * @return if mode changes should trigger persisting.
     * @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;
        final boolean persistable = (modeFlags & Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION) != 0;
        modeFlags &= (Intent.FLAG_GRANT_READ_URI_PERMISSION
        modeFlags &= (Intent.FLAG_GRANT_READ_URI_PERMISSION
                | Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
                | Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
@@ -193,7 +193,7 @@ final class UriPermission {
                persistedModeFlags &= ~Intent.FLAG_GRANT_READ_URI_PERMISSION;
                persistedModeFlags &= ~Intent.FLAG_GRANT_READ_URI_PERMISSION;
            }
            }
            globalModeFlags &= ~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;
                ownedModeFlags &= ~Intent.FLAG_GRANT_READ_URI_PERMISSION;
                for (UriPermissionOwner r : mReadOwners) {
                for (UriPermissionOwner r : mReadOwners) {
                    r.removeReadPermission(this);
                    r.removeReadPermission(this);
@@ -207,7 +207,7 @@ final class UriPermission {
                persistedModeFlags &= ~Intent.FLAG_GRANT_WRITE_URI_PERMISSION;
                persistedModeFlags &= ~Intent.FLAG_GRANT_WRITE_URI_PERMISSION;
            }
            }
            globalModeFlags &= ~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;
                ownedModeFlags &= ~Intent.FLAG_GRANT_WRITE_URI_PERMISSION;
                for (UriPermissionOwner r : mWriteOwners) {
                for (UriPermissionOwner r : mWriteOwners) {
                    r.removeWritePermission(this);
                    r.removeWritePermission(this);