summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Hai Zhang <zhanghai@google.com> 2022-03-02 02:10:15 +0000
committer Hai Zhang <zhanghai@google.com> 2022-03-02 10:57:09 +0000
commit01221cb71625d2dc095e02a97554ca13b64c76ec (patch)
tree08a25e11919ed3de57680d7bd4427835cac18ffd
parentdde7ea23aa62c4fa4add29bdd85d20ecf4b7cd18 (diff)
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
-rw-r--r--services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java28
1 files 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 def0ed568030..351c5be5e791 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()) {