From 01221cb71625d2dc095e02a97554ca13b64c76ec Mon Sep 17 00:00:00 2001
From: Hai Zhang <zhanghai@google.com>
Date: Wed, 2 Mar 2022 02:10:15 +0000
Subject: [PATCH] Fix potential deadlock in restorePermissionState().

The call to retrieve all shared user packages was on the
PackageSetting object and was lockless, however it was replaced with a
PackageManagerInternal call in ag/16740473 and the method may take a
lock. This creates a potential deadlock because we are already holding
our own lock in some cases, and we shouldn't allow calling another
system service while holding it.

This change makes revokeUnusedSharedUserPermissionsLocked() re-use the
uidRequestedPermissions that we calculated earlier, and the behavior
of the method remains unchanged.

Bug: 216207402
Test: presubmit
Change-Id: I4b031cb670a6921bfa1425fff65f58cd28af576e
---
 .../PermissionManagerServiceImpl.java         | 28 +++----------------
 1 file changed, 4 insertions(+), 24 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 def0ed5680308..351c5be5e7910 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java
@@ -2611,8 +2611,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
                         // the runtime ones are written only if changed. The only cases of
                         // changed runtime permissions here are promotion of an install to
                         // runtime and revocation of a runtime from a shared user.
-                        if (revokeUnusedSharedUserPermissionsLocked(
-                                mPackageManagerInt.getSharedUserPackages(ps.getSharedUserAppId()),
+                        if (revokeUnusedSharedUserPermissionsLocked(uidRequestedPermissions,
                                 uidState)) {
                             updatedUserIds = ArrayUtils.appendInt(updatedUserIds, userId);
                             runtimePermissionsRevoked = true;
@@ -3828,27 +3827,8 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
 
     @GuardedBy("mLock")
     private boolean revokeUnusedSharedUserPermissionsLocked(
-            ArraySet<PackageStateInternal> pkgList, UidPermissionState uidState) {
-        // Collect all used permissions in the UID
-        final ArraySet<String> usedPermissions = new ArraySet<>();
-        if (pkgList == null || pkgList.size() == 0) {
-            return false;
-        }
-        for (PackageStateInternal pkgState : pkgList) {
-            final AndroidPackageApi pkg = pkgState.getAndroidPackage();
-            if (pkg.getRequestedPermissions().isEmpty()) {
-                continue;
-            }
-            final int requestedPermCount = pkg.getRequestedPermissions().size();
-            for (int j = 0; j < requestedPermCount; j++) {
-                String permission = pkg.getRequestedPermissions().get(j);
-                Permission bp = mRegistry.getPermission(permission);
-                if (bp != null) {
-                    usedPermissions.add(permission);
-                }
-            }
-        }
-
+            @NonNull Collection<String> uidRequestedPermissions,
+            @NonNull UidPermissionState uidState) {
         boolean runtimePermissionChanged = false;
 
         // Prune permissions
@@ -3856,7 +3836,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt
         final int permissionStatesSize = permissionStates.size();
         for (int i = permissionStatesSize - 1; i >= 0; i--) {
             PermissionState permissionState = permissionStates.get(i);
-            if (!usedPermissions.contains(permissionState.getName())) {
+            if (!uidRequestedPermissions.contains(permissionState.getName())) {
                 Permission bp = mRegistry.getPermission(permissionState.getName());
                 if (bp != null) {
                     if (uidState.removePermissionState(bp.getName()) && bp.isRuntime()) {