Properly fix revokePermissionsNoLongerImplicitLocked() for shared UIDs.

This is actually a follow up to ag/13938162 that will properly fix the
shared UID handling.

If there are multiple packages in the same shared UID, the target SDK
version of the UID should be the lowest target SDK version of those
packages. This is also confirmed by existing code in
PermissionManagerService and PackageManagerService.

However, the current implicit permission revocation is only reading
from the implicit permissions of the particular package, which is
determined during package parsing according to the target SDK version
of that package. So if a permission in one package in the shared UID
is becoming explicit, it breaks the other package if it still relies
on the permission being implicitly. And in reality, it's hard to make
sure the other package has been updated to handle this situation.

So in order to properly fix this, we need to collect all the implicit
permissions in this shared UID and pass that to the revocation
logic. This does mean we will be doing exactly the same thing again
when we are handling other packages' permission state, but that won't
be a big issue, whereas the alternative is to refactor the code to
handle permission state by UID which is way too risky for T or
backports. In the proposed new permission subsystem, we will indeed
handle permissions by UID though.

This change also fixes a potential deadlock bug introduce by
PackageManagerService refactor - we should never call into other
components while holding our mLock.

Bug: 201263297
Test: presubmit
Change-Id: I8315dc27b703d819e8970134d5cf17b0edbc22dc

1 files changed, 38 insertions, 32 deletions

diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java
index 87494a62b625..def0ed568030 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java
@@ -2536,33 +2536,38 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
             }
         }
 
+        Collection<String> uidRequestedPermissions;
+        Collection<String> uidImplicitPermissions;
+        int uidTargetSdkVersion;
+        if (!ps.hasSharedUser()) {
+            uidRequestedPermissions = pkg.getRequestedPermissions();
+            uidImplicitPermissions = pkg.getImplicitPermissions();
+            uidTargetSdkVersion = pkg.getTargetSdkVersion();
+        } else {
+            uidRequestedPermissions = new ArraySet<>();
+            uidImplicitPermissions = new ArraySet<>();
+            uidTargetSdkVersion = Build.VERSION_CODES.CUR_DEVELOPMENT;
+            final ArraySet<PackageStateInternal> packages =
+                    mPackageManagerInt.getSharedUserPackages(ps.getSharedUserAppId());
+            int packagesSize = packages.size();
+            for (int i = 0; i < packagesSize; i++) {
+                AndroidPackageApi sharedUserPackage =
+                        packages.valueAt(i).getAndroidPackage();
+                uidRequestedPermissions.addAll(
+                        sharedUserPackage.getRequestedPermissions());
+                uidImplicitPermissions.addAll(
+                        sharedUserPackage.getImplicitPermissions());
+                uidTargetSdkVersion = Math.min(uidTargetSdkVersion,
+                        sharedUserPackage.getTargetSdkVersion());
+            }
+        }
+
         synchronized (mLock) {
             for (final int userId : userIds) {
                 final UserPermissionState userState = mState.getOrCreateUserState(userId);
                 final UidPermissionState uidState = userState.getOrCreateUidState(ps.getAppId());
 
                 if (uidState.isMissing()) {
-                    Collection<String> uidRequestedPermissions;
-                    int targetSdkVersion;
-                    if (!ps.hasSharedUser()) {
-                        uidRequestedPermissions = pkg.getRequestedPermissions();
-                        targetSdkVersion = pkg.getTargetSdkVersion();
-                    } else {
-                        uidRequestedPermissions = new ArraySet<>();
-                        targetSdkVersion = Build.VERSION_CODES.CUR_DEVELOPMENT;
-                        final ArraySet<PackageStateInternal> packages =
-                                mPackageManagerInt.getSharedUserPackages(ps.getSharedUserAppId());
-                        int packagesSize = packages.size();
-                        for (int i = 0; i < packagesSize; i++) {
-                            AndroidPackageApi sharedUserPackage =
-                                    packages.valueAt(i).getAndroidPackage();
-                            uidRequestedPermissions.addAll(
-                                    sharedUserPackage.getRequestedPermissions());
-                            targetSdkVersion = Math.min(targetSdkVersion,
-                                    sharedUserPackage.getTargetSdkVersion());
-                        }
-                    }
-
                     for (String permissionName : uidRequestedPermissions) {
                         Permission permission = mRegistry.getPermission(permissionName);
                         if (permission == null) {
@@ -2576,7 +2581,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
                                         FLAG_PERMISSION_RESTRICTION_UPGRADE_EXEMPT,
                                         FLAG_PERMISSION_RESTRICTION_UPGRADE_EXEMPT);
                             }
-                            if (targetSdkVersion < Build.VERSION_CODES.M) {
+                            if (uidTargetSdkVersion < Build.VERSION_CODES.M) {
                                 uidState.updatePermissionFlags(permission,
                                         PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED
                                                 | PackageManager.FLAG_PERMISSION_REVOKED_COMPAT,
@@ -2909,8 +2914,9 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
                     userState.setInstallPermissionsFixed(ps.getPackageName(), true);
                 }
 
-                updatedUserIds = revokePermissionsNoLongerImplicitLocked(uidState, pkg,
-                        userId, updatedUserIds);
+                updatedUserIds = revokePermissionsNoLongerImplicitLocked(uidState,
+                        pkg.getPackageName(), uidImplicitPermissions, uidTargetSdkVersion, userId,
+                        updatedUserIds);
                 updatedUserIds = setInitialGrantForNewImplicitPermissionsLocked(origState,
                         uidState, pkg, newImplicitPermissions, userId, updatedUserIds);
             }
@@ -2947,7 +2953,9 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
      * {@link PackageManager#FLAG_PERMISSION_REVOKE_WHEN_REQUESTED} set.
      *
      * @param ps The state of the permissions of the package
-     * @param pkg The package that is currently looked at
+     * @param packageName The name of the package
+     * @param uidImplicitPermissions The implicit permissions of all packages in the UID
+     * @param uidTargetSdkVersion The lowest target SDK version of all packages in the UID
      * @param userIds All user IDs in the system, must be passed in because this method is locked
      * @param updatedUserIds a list of user ids that needs to be amended if the permission state
      *                       for a user is changed.
@@ -2957,14 +2965,12 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
     @NonNull
     @GuardedBy("mLock")
     private int[] revokePermissionsNoLongerImplicitLocked(@NonNull UidPermissionState ps,
-            @NonNull AndroidPackage pkg, int userId, @NonNull int[] updatedUserIds) {
-        String pkgName = pkg.getPackageName();
-        boolean supportsRuntimePermissions = pkg.getTargetSdkVersion()
-                >= Build.VERSION_CODES.M;
+            @NonNull String packageName, @NonNull Collection<String> uidImplicitPermissions,
+            int uidTargetSdkVersion, int userId, @NonNull int[] updatedUserIds) {
+        boolean supportsRuntimePermissions = uidTargetSdkVersion >= Build.VERSION_CODES.M;
 
         for (String permission : ps.getGrantedPermissions()) {
-            if (pkg.getRequestedPermissions().contains(permission)
-                    && !pkg.getImplicitPermissions().contains(permission)) {
+            if (!uidImplicitPermissions.contains(permission)) {
                 Permission bp = mRegistry.getPermission(permission);
                 if (bp != null && bp.isRuntime()) {
                     int flags = ps.getPermissionFlags(permission);
@@ -2991,7 +2997,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
                             if (ps.revokePermission(bp)) {
                                 if (DEBUG_PERMISSIONS) {
                                     Slog.i(TAG, "Revoking runtime permission "
-                                            + permission + " for " + pkgName
+                                            + permission + " for " + packageName
                                             + " as it is now requested");
                                 }
                             }