From a71845ef412bcce20d2e43afc3854233c4a3015a Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Tue, 15 Sep 2020 22:46:57 -0700 Subject: Use the service lock for permission state. Remove locks inside permission state classes, and make sure that code using mState or calling getUidStateLocked() is under mLock. grantRequestedRuntimePermissionsForUser() and setWhitelistedRestrictedPermissionsForUsers() were never called under the package manager lock, so it's also fine to go through each permission without holding mLock over the entire loop. Bug: 158736025 Test: presubmit Change-Id: I19d7daf76ccdc69bb1494f1b1b45a7e8a05befeb --- .../server/pm/permission/BasePermission.java | 12 - .../pm/permission/DevicePermissionState.java | 45 +- .../pm/permission/PermissionManagerService.java | 692 +++++++++++---------- .../server/pm/permission/UidPermissionState.java | 228 +++---- .../server/pm/permission/UserPermissionState.java | 47 +- 5 files changed, 493 insertions(+), 531 deletions(-) diff --git a/services/core/java/com/android/server/pm/permission/BasePermission.java b/services/core/java/com/android/server/pm/permission/BasePermission.java index eb06bf953059..c08601740158 100644 --- a/services/core/java/com/android/server/pm/permission/BasePermission.java +++ b/services/core/java/com/android/server/pm/permission/BasePermission.java @@ -433,18 +433,6 @@ public final class BasePermission { throw new SecurityException("No permission tree found for " + permName); } - public void enforceDeclaredUsedAndRuntimeOrDevelopment(AndroidPackage pkg, - UidPermissionState uidState) { - if (!uidState.hasPermissionState(name) && !pkg.getRequestedPermissions().contains(name)) { - throw new SecurityException("Package " + pkg.getPackageName() - + " has not requested permission " + name); - } - if (!isRuntime() && !isDevelopment()) { - throw new SecurityException("Permission " + name + " requested by " - + pkg.getPackageName() + " is not a changeable permission type"); - } - } - private static BasePermission findPermissionTree( Collection permissionTrees, String permName) { for (BasePermission bp : permissionTrees) { diff --git a/services/core/java/com/android/server/pm/permission/DevicePermissionState.java b/services/core/java/com/android/server/pm/permission/DevicePermissionState.java index b9456acfced5..18936dda2ecc 100644 --- a/services/core/java/com/android/server/pm/permission/DevicePermissionState.java +++ b/services/core/java/com/android/server/pm/permission/DevicePermissionState.java @@ -21,57 +21,38 @@ import android.annotation.Nullable; import android.annotation.UserIdInt; import android.util.SparseArray; -import com.android.internal.annotations.GuardedBy; - /** * Permission state for this device. */ public final class DevicePermissionState { - @GuardedBy("mLock") - @NonNull private final SparseArray mUserStates = new SparseArray<>(); - @NonNull - private final Object mLock; - - public DevicePermissionState(@NonNull Object lock) { - mLock = lock; - } - @Nullable public UserPermissionState getUserState(@UserIdInt int userId) { - synchronized (mLock) { - return mUserStates.get(userId); - } + return mUserStates.get(userId); } @NonNull public UserPermissionState getOrCreateUserState(@UserIdInt int userId) { - synchronized (mLock) { - UserPermissionState userState = mUserStates.get(userId); - if (userState == null) { - userState = new UserPermissionState(mLock); - mUserStates.put(userId, userState); - } - return userState; + UserPermissionState userState = mUserStates.get(userId); + if (userState == null) { + userState = new UserPermissionState(); + mUserStates.put(userId, userState); } + return userState; } public void removeUserState(@UserIdInt int userId) { - synchronized (mLock) { - mUserStates.delete(userId); - } + mUserStates.delete(userId); } public int[] getUserIds() { - synchronized (mLock) { - final int userStatesSize = mUserStates.size(); - final int[] userIds = new int[userStatesSize]; - for (int i = 0; i < userStatesSize; i++) { - final int userId = mUserStates.keyAt(i); - userIds[i] = userId; - } - return userIds; + final int userStatesSize = mUserStates.size(); + final int[] userIds = new int[userStatesSize]; + for (int i = 0; i < userStatesSize; i++) { + final int userId = mUserStates.keyAt(i); + userIds[i] = userId; } + return userIds; } } diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index afecd8e92646..132535602b75 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -216,8 +216,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { /** Internal connection to the user manager */ private final UserManagerInternal mUserManagerInt; + @GuardedBy("mLock") @NonNull - private final DevicePermissionState mState; + private final DevicePermissionState mState = new DevicePermissionState(); /** Permission controller: User space permission management */ private PermissionControllerManager mPermissionControllerManager; @@ -386,7 +387,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { mPackageManagerInt = LocalServices.getService(PackageManagerInternal.class); mUserManagerInt = LocalServices.getService(UserManagerInternal.class); mSettings = new PermissionSettings(mLock); - mState = new DevicePermissionState(mLock); mAppOpsManager = context.getSystemService(AppOpsManager.class); mHandlerThread = new ServiceThread(TAG, @@ -665,20 +665,23 @@ public class PermissionManagerService extends IPermissionManager.Stub { if (pkg == null) { return 0; } + if (mPackageManagerInt.filterAppAccess(pkg, callingUid, userId)) { + return 0; + } + synchronized (mLock) { if (mSettings.getPermissionLocked(permName) == null) { return 0; } + + final UidPermissionState uidState = getUidStateLocked(pkg, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + packageName + " and user " + userId); + return 0; + } + + return uidState.getPermissionFlags(permName); } - if (mPackageManagerInt.filterAppAccess(pkg, callingUid, userId)) { - return 0; - } - final UidPermissionState uidState = getUidState(pkg, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + packageName + " and user " + userId); - return 0; - } - return uidState.getPermissionFlags(permName); } @Override @@ -772,52 +775,52 @@ public class PermissionManagerService extends IPermissionManager.Stub { throw new IllegalArgumentException("Unknown package: " + packageName); } + boolean isRequested = false; + // Fast path, the current package has requested the permission. + if (pkg.getRequestedPermissions().contains(permName)) { + isRequested = true; + } + if (!isRequested) { + // Slow path, go through all shared user packages. + String[] sharedUserPackageNames = + mPackageManagerInt.getSharedUserPackagesForPackage(packageName, userId); + for (String sharedUserPackageName : sharedUserPackageNames) { + AndroidPackage sharedUserPkg = mPackageManagerInt.getPackage( + sharedUserPackageName); + if (sharedUserPkg != null + && sharedUserPkg.getRequestedPermissions().contains(permName)) { + isRequested = true; + break; + } + } + } + final BasePermission bp; + final boolean permissionUpdated; synchronized (mLock) { bp = mSettings.getPermissionLocked(permName); - } - if (bp == null) { - throw new IllegalArgumentException("Unknown permission: " + permName); - } - - if (bp.isInstallerExemptIgnored()) { - flagValues &= ~FLAG_PERMISSION_RESTRICTION_INSTALLER_EXEMPT; - } + if (bp == null) { + throw new IllegalArgumentException("Unknown permission: " + permName); + } - final UidPermissionState uidState = getUidState(pkg, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + packageName + " and user " + userId); - return; - } + if (bp.isInstallerExemptIgnored()) { + flagValues &= ~FLAG_PERMISSION_RESTRICTION_INSTALLER_EXEMPT; + } - final boolean hadState = uidState.getPermissionState(permName) != null; - if (!hadState) { - boolean isRequested = false; - // Fast path, the current package has requested the permission. - if (pkg.getRequestedPermissions().contains(permName)) { - isRequested = true; - } - if (!isRequested) { - // Slow path, go through all shared user packages. - String[] sharedUserPackageNames = - mPackageManagerInt.getSharedUserPackagesForPackage(packageName, userId); - for (String sharedUserPackageName : sharedUserPackageNames) { - AndroidPackage sharedUserPkg = mPackageManagerInt.getPackage( - sharedUserPackageName); - if (sharedUserPkg != null - && sharedUserPkg.getRequestedPermissions().contains(permName)) { - isRequested = true; - break; - } - } + final UidPermissionState uidState = getUidStateLocked(pkg, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + packageName + " and user " + userId); + return; } - if (!isRequested) { + + if (!uidState.hasPermissionState(permName) && !isRequested) { Log.e(TAG, "Permission " + permName + " isn't requested by package " + packageName); return; } + + permissionUpdated = uidState.updatePermissionFlags(bp, flagMask, flagValues); } - final boolean permissionUpdated = - uidState.updatePermissionFlags(bp, flagMask, flagValues); + if (permissionUpdated && bp.isRuntime()) { notifyRuntimePermissionStateChanged(packageName, userId); } @@ -861,14 +864,17 @@ public class PermissionManagerService extends IPermissionManager.Stub { final boolean[] changed = new boolean[1]; mPackageManagerInt.forEachPackage(pkg -> { - final UidPermissionState uidState = getUidState(pkg, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + " and user " - + userId); - return; + synchronized (mLock) { + final UidPermissionState uidState = getUidStateLocked(pkg, userId); + if (uidState == null) { + Slog.e(TAG, + "Missing permissions state for " + pkg.getPackageName() + " and user " + + userId); + return; + } + changed[0] |= uidState.updatePermissionFlagsForAllPermissions( + effectiveFlagMask, effectiveFlagValues); } - changed[0] |= uidState.updatePermissionFlagsForAllPermissions( - effectiveFlagMask, effectiveFlagValues); mOnPermissionChangeListeners.onPermissionsChanged(pkg.getUid()); }); @@ -920,33 +926,37 @@ public class PermissionManagerService extends IPermissionManager.Stub { } final int uid = UserHandle.getUid(userId, pkg.getUid()); - final UidPermissionState uidState = getUidState(pkg, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + " and user " - + userId); - return PackageManager.PERMISSION_DENIED; - } + final boolean isInstantApp = mPackageManagerInt.getInstantAppPackageName(uid) != null; - if (checkSinglePermissionInternal(uid, uidState, permissionName)) { - return PackageManager.PERMISSION_GRANTED; - } + synchronized (mLock) { + final UidPermissionState uidState = getUidStateLocked(pkg, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + " and user " + + userId); + return PackageManager.PERMISSION_DENIED; + } - final String fullerPermissionName = FULLER_PERMISSION_MAP.get(permissionName); - if (fullerPermissionName != null - && checkSinglePermissionInternal(uid, uidState, fullerPermissionName)) { - return PackageManager.PERMISSION_GRANTED; + if (checkSinglePermissionInternalLocked(uidState, permissionName, isInstantApp)) { + return PackageManager.PERMISSION_GRANTED; + } + + final String fullerPermissionName = FULLER_PERMISSION_MAP.get(permissionName); + if (fullerPermissionName != null && checkSinglePermissionInternalLocked(uidState, + fullerPermissionName, isInstantApp)) { + return PackageManager.PERMISSION_GRANTED; + } } return PackageManager.PERMISSION_DENIED; } - private boolean checkSinglePermissionInternal(int uid, - @NonNull UidPermissionState uidState, @NonNull String permissionName) { + private boolean checkSinglePermissionInternalLocked(@NonNull UidPermissionState uidState, + @NonNull String permissionName, boolean isInstantApp) { if (!uidState.isPermissionGranted(permissionName)) { return false; } - if (mPackageManagerInt.getInstantAppPackageName(uid) != null) { + if (isInstantApp) { return mSettings.isPermissionInstant(permissionName); } @@ -994,24 +1004,25 @@ public class PermissionManagerService extends IPermissionManager.Stub { return checkPermissionInternal(pkg, false, permissionName, userId); } - if (checkSingleUidPermissionInternal(uid, permissionName)) { - return PackageManager.PERMISSION_GRANTED; - } + synchronized (mLock) { + if (checkSingleUidPermissionInternalLocked(uid, permissionName)) { + return PackageManager.PERMISSION_GRANTED; + } - final String fullerPermissionName = FULLER_PERMISSION_MAP.get(permissionName); - if (fullerPermissionName != null - && checkSingleUidPermissionInternal(uid, fullerPermissionName)) { - return PackageManager.PERMISSION_GRANTED; + final String fullerPermissionName = FULLER_PERMISSION_MAP.get(permissionName); + if (fullerPermissionName != null + && checkSingleUidPermissionInternalLocked(uid, fullerPermissionName)) { + return PackageManager.PERMISSION_GRANTED; + } } return PackageManager.PERMISSION_DENIED; } - private boolean checkSingleUidPermissionInternal(int uid, @NonNull String permissionName) { - synchronized (mLock) { - ArraySet permissions = mSystemPermissions.get(uid); - return permissions != null && permissions.contains(permissionName); - } + private boolean checkSingleUidPermissionInternalLocked(int uid, + @NonNull String permissionName) { + ArraySet permissions = mSystemPermissions.get(uid); + return permissions != null && permissions.contains(permissionName); } @Override @@ -1137,42 +1148,45 @@ public class PermissionManagerService extends IPermissionManager.Stub { final long identity = Binder.clearCallingIdentity(); try { - final UidPermissionState uidState = getUidState(pkg, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + packageName + " and user " + userId); - return null; - } + synchronized (mLock) { + final UidPermissionState uidState = getUidStateLocked(pkg, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + packageName + " and user " + + userId); + return null; + } - int queryFlags = 0; - if ((flags & PackageManager.FLAG_PERMISSION_WHITELIST_SYSTEM) != 0) { - queryFlags |= FLAG_PERMISSION_RESTRICTION_SYSTEM_EXEMPT; - } - if ((flags & PackageManager.FLAG_PERMISSION_WHITELIST_UPGRADE) != 0) { - queryFlags |= FLAG_PERMISSION_RESTRICTION_UPGRADE_EXEMPT; - } - if ((flags & PackageManager.FLAG_PERMISSION_WHITELIST_INSTALLER) != 0) { - queryFlags |= FLAG_PERMISSION_RESTRICTION_INSTALLER_EXEMPT; - } - if ((flags & PackageManager.FLAG_PERMISSION_ALLOWLIST_ROLE) != 0) { - queryFlags |= FLAG_PERMISSION_RESTRICTION_ROLE_EXEMPT; - } + int queryFlags = 0; + if ((flags & PackageManager.FLAG_PERMISSION_WHITELIST_SYSTEM) != 0) { + queryFlags |= FLAG_PERMISSION_RESTRICTION_SYSTEM_EXEMPT; + } + if ((flags & PackageManager.FLAG_PERMISSION_WHITELIST_UPGRADE) != 0) { + queryFlags |= FLAG_PERMISSION_RESTRICTION_UPGRADE_EXEMPT; + } + if ((flags & PackageManager.FLAG_PERMISSION_WHITELIST_INSTALLER) != 0) { + queryFlags |= FLAG_PERMISSION_RESTRICTION_INSTALLER_EXEMPT; + } + if ((flags & PackageManager.FLAG_PERMISSION_ALLOWLIST_ROLE) != 0) { + queryFlags |= FLAG_PERMISSION_RESTRICTION_ROLE_EXEMPT; + } - ArrayList whitelistedPermissions = null; + ArrayList whitelistedPermissions = null; - final int permissionCount = ArrayUtils.size(pkg.getRequestedPermissions()); - for (int i = 0; i < permissionCount; i++) { - final String permissionName = pkg.getRequestedPermissions().get(i); - final int currentFlags = - uidState.getPermissionFlags(permissionName); - if ((currentFlags & queryFlags) != 0) { - if (whitelistedPermissions == null) { - whitelistedPermissions = new ArrayList<>(); + final int permissionCount = ArrayUtils.size(pkg.getRequestedPermissions()); + for (int i = 0; i < permissionCount; i++) { + final String permissionName = pkg.getRequestedPermissions().get(i); + final int currentFlags = + uidState.getPermissionFlags(permissionName); + if ((currentFlags & queryFlags) != 0) { + if (whitelistedPermissions == null) { + whitelistedPermissions = new ArrayList<>(); + } + whitelistedPermissions.add(permissionName); } - whitelistedPermissions.add(permissionName); } - } - return whitelistedPermissions; + return whitelistedPermissions; + } } finally { Binder.restoreCallingIdentity(identity); } @@ -1437,12 +1451,15 @@ public class PermissionManagerService extends IPermissionManager.Stub { "grantRuntimePermission"); final AndroidPackage pkg = mPackageManagerInt.getPackage(packageName); - final PackageSetting ps = (PackageSetting) mPackageManagerInt.getPackageSetting( - packageName); + final PackageSetting ps = mPackageManagerInt.getPackageSetting(packageName); if (pkg == null || ps == null) { Log.e(TAG, "Unknown package: " + packageName); return; } + if (mPackageManagerInt.filterAppAccess(pkg, callingUid, userId)) { + throw new IllegalArgumentException("Unknown package: " + packageName); + } + final BasePermission bp; synchronized (mLock) { bp = mSettings.getPermissionLocked(permName); @@ -1450,46 +1467,17 @@ public class PermissionManagerService extends IPermissionManager.Stub { if (bp == null) { throw new IllegalArgumentException("Unknown permission: " + permName); } - if (mPackageManagerInt.filterAppAccess(pkg, callingUid, userId)) { - throw new IllegalArgumentException("Unknown package: " + packageName); - } - final UidPermissionState uidState = getUidState(pkg, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + " and user " - + userId); - return; + if (!(bp.isRuntime() || bp.isDevelopment())) { + throw new SecurityException("Permission " + permName + " requested by " + + pkg.getPackageName() + " is not a changeable permission type"); } - bp.enforceDeclaredUsedAndRuntimeOrDevelopment(pkg, uidState); - // If a permission review is required for legacy apps we represent // their permissions as always granted runtime ones since we need // to keep the review required permission flag per user while an // install permission's state is shared across all users. - if (pkg.getTargetSdkVersion() < Build.VERSION_CODES.M - && bp.isRuntime()) { - return; - } - - final int uid = UserHandle.getUid(userId, UserHandle.getAppId(pkg.getUid())); - - final int flags = uidState.getPermissionFlags(permName); - if ((flags & PackageManager.FLAG_PERMISSION_SYSTEM_FIXED) != 0) { - Log.e(TAG, "Cannot grant system fixed permission " - + permName + " for package " + packageName); - return; - } - if (!overridePolicy && (flags & PackageManager.FLAG_PERMISSION_POLICY_FIXED) != 0) { - Log.e(TAG, "Cannot grant policy fixed permission " - + permName + " for package " + packageName); - return; - } - - if (bp.isHardRestricted() - && (flags & PackageManager.FLAGS_PERMISSION_RESTRICTION_ANY_EXEMPT) == 0) { - Log.e(TAG, "Cannot grant hard restricted non-exempt permission " - + permName + " for package " + packageName); + if (pkg.getTargetSdkVersion() < Build.VERSION_CODES.M && bp.isRuntime()) { return; } @@ -1501,36 +1489,61 @@ public class PermissionManagerService extends IPermissionManager.Stub { return; } - if (bp.isDevelopment()) { - // Development permissions must be handled specially, since they are not - // normal runtime permissions. For now they apply to all users. - // TODO(zhanghai): We are breaking the behavior above by making all permission state - // per-user. It isn't documented behavior and relatively rarely used anyway. - if (uidState.grantPermission(bp)) { - if (callback != null) { - callback.onInstallPermissionGranted(); - } + synchronized (mLock) { + final UidPermissionState uidState = getUidStateLocked(pkg, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + " and user " + + userId); + return; } - return; - } - if (ps.getInstantApp(userId) && !bp.isInstant()) { - throw new SecurityException("Cannot grant non-ephemeral permission" - + permName + " for package " + packageName); - } + if (!(uidState.hasPermissionState(permName) + || pkg.getRequestedPermissions().contains(permName))) { + throw new SecurityException("Package " + pkg.getPackageName() + + " has not requested permission " + permName); + } - if (pkg.getTargetSdkVersion() < Build.VERSION_CODES.M) { - Slog.w(TAG, "Cannot grant runtime permission to a legacy app"); - return; - } + final int flags = uidState.getPermissionFlags(permName); + if ((flags & PackageManager.FLAG_PERMISSION_SYSTEM_FIXED) != 0) { + Log.e(TAG, "Cannot grant system fixed permission " + + permName + " for package " + packageName); + return; + } + if (!overridePolicy && (flags & PackageManager.FLAG_PERMISSION_POLICY_FIXED) != 0) { + Log.e(TAG, "Cannot grant policy fixed permission " + + permName + " for package " + packageName); + return; + } - if (!uidState.grantPermission(bp)) { - return; - } + if (bp.isHardRestricted() + && (flags & PackageManager.FLAGS_PERMISSION_RESTRICTION_ANY_EXEMPT) == 0) { + Log.e(TAG, "Cannot grant hard restricted non-exempt permission " + + permName + " for package " + packageName); + return; + } - if (bp.hasGids()) { - if (callback != null) { - callback.onGidsChanged(UserHandle.getAppId(pkg.getUid()), userId); + if (bp.isDevelopment()) { + // Development permissions must be handled specially, since they are not + // normal runtime permissions. For now they apply to all users. + // TODO(zhanghai): We are breaking the behavior above by making all permission state + // per-user. It isn't documented behavior and relatively rarely used anyway. + if (!uidState.grantPermission(bp)) { + return; + } + } else { + if (ps.getInstantApp(userId) && !bp.isInstant()) { + throw new SecurityException("Cannot grant non-ephemeral permission" + permName + + " for package " + packageName); + } + + if (pkg.getTargetSdkVersion() < Build.VERSION_CODES.M) { + Slog.w(TAG, "Cannot grant runtime permission to a legacy app"); + return; + } + + if (!uidState.grantPermission(bp)) { + return; + } } } @@ -1538,8 +1551,16 @@ public class PermissionManagerService extends IPermissionManager.Stub { logPermission(MetricsEvent.ACTION_PERMISSION_GRANTED, permName, packageName); } + final int uid = UserHandle.getUid(userId, UserHandle.getAppId(pkg.getUid())); if (callback != null) { - callback.onPermissionGranted(uid, userId); + if (bp.isDevelopment()) { + callback.onInstallPermissionGranted(); + } else { + callback.onPermissionGranted(uid, userId); + } + if (bp.hasGids()) { + callback.onGidsChanged(UserHandle.getAppId(pkg.getUid()), userId); + } } if (bp.isRuntime()) { @@ -1593,56 +1614,57 @@ public class PermissionManagerService extends IPermissionManager.Stub { if (mPackageManagerInt.filterAppAccess(pkg, callingUid, userId)) { throw new IllegalArgumentException("Unknown package: " + packageName); } - final BasePermission bp = mSettings.getPermissionLocked(permName); + final BasePermission bp = mSettings.getPermission(permName); if (bp == null) { throw new IllegalArgumentException("Unknown permission: " + permName); } - final UidPermissionState uidState = getUidState(pkg, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + " and user " - + userId); - return; + if (!(bp.isRuntime() || bp.isDevelopment())) { + throw new SecurityException("Permission " + permName + " requested by " + + pkg.getPackageName() + " is not a changeable permission type"); } - bp.enforceDeclaredUsedAndRuntimeOrDevelopment(pkg, uidState); + synchronized (mLock) { + final UidPermissionState uidState = getUidStateLocked(pkg, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + " and user " + + userId); + return; + } - // If a permission review is required for legacy apps we represent - // their permissions as always granted runtime ones since we need - // to keep the review required permission flag per user while an - // install permission's state is shared across all users. - if (pkg.getTargetSdkVersion() < Build.VERSION_CODES.M - && bp.isRuntime()) { - return; - } + if (!(uidState.hasPermissionState(permName) + || pkg.getRequestedPermissions().contains(permName))) { + throw new SecurityException("Package " + pkg.getPackageName() + + " has not requested permission " + permName); + } - final int flags = uidState.getPermissionFlags(permName); - // Only the system may revoke SYSTEM_FIXED permissions. - if ((flags & PackageManager.FLAG_PERMISSION_SYSTEM_FIXED) != 0 - && UserHandle.getCallingAppId() != Process.SYSTEM_UID) { - throw new SecurityException("Non-System UID cannot revoke system fixed permission " - + permName + " for package " + packageName); - } - if (!overridePolicy && (flags & PackageManager.FLAG_PERMISSION_POLICY_FIXED) != 0) { - throw new SecurityException("Cannot revoke policy fixed permission " - + permName + " for package " + packageName); - } + // If a permission review is required for legacy apps we represent + // their permissions as always granted runtime ones since we need + // to keep the review required permission flag per user while an + // install permission's state is shared across all users. + if (pkg.getTargetSdkVersion() < Build.VERSION_CODES.M && bp.isRuntime()) { + return; + } + + final int flags = uidState.getPermissionFlags(permName); + // Only the system may revoke SYSTEM_FIXED permissions. + if ((flags & PackageManager.FLAG_PERMISSION_SYSTEM_FIXED) != 0 + && UserHandle.getCallingAppId() != Process.SYSTEM_UID) { + throw new SecurityException("Non-System UID cannot revoke system fixed permission " + + permName + " for package " + packageName); + } + if (!overridePolicy && (flags & PackageManager.FLAG_PERMISSION_POLICY_FIXED) != 0) { + throw new SecurityException("Cannot revoke policy fixed permission " + + permName + " for package " + packageName); + } - if (bp.isDevelopment()) { // Development permissions must be handled specially, since they are not // normal runtime permissions. For now they apply to all users. // TODO(zhanghai): We are breaking the behavior above by making all permission state // per-user. It isn't documented behavior and relatively rarely used anyway. - if (uidState.revokePermission(bp)) { - if (callback != null) { - mDefaultPermissionCallback.onInstallPermissionRevoked(); - } + if (!uidState.revokePermission(bp)) { + return; } - return; - } - - if (!uidState.revokePermission(bp)) { - return; } if (bp.isRuntime()) { @@ -1650,8 +1672,12 @@ public class PermissionManagerService extends IPermissionManager.Stub { } if (callback != null) { - callback.onPermissionRevoked(UserHandle.getUid(userId, - UserHandle.getAppId(pkg.getUid())), userId, reason); + if (bp.isDevelopment()) { + mDefaultPermissionCallback.onInstallPermissionRevoked(); + } else { + callback.onPermissionRevoked(UserHandle.getUid(userId, + UserHandle.getAppId(pkg.getUid())), userId, reason); + } } if (bp.isRuntime()) { @@ -1686,7 +1712,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { * @param pkg The package for which to reset. * @param userId The device user for which to do a reset. */ - @GuardedBy("mLock") private void resetRuntimePermissionsInternal(final AndroidPackage pkg, final int userId) { final String packageName = pkg.getPackageName(); @@ -2323,7 +2348,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { // Assume by default that we did not install this permission into the system. p.setFlags(p.getFlags() & ~PermissionInfo.FLAG_INSTALLED); - synchronized (PermissionManagerService.this.mLock) { + synchronized (mLock) { // Now that permission groups have a special meaning, we ignore permission // groups for legacy apps to prevent unexpected behavior. In particular, // permissions for one app being granted to someone just because they happen @@ -2463,31 +2488,35 @@ public class PermissionManagerService extends IPermissionManager.Stub { if (ps == null) { return Collections.emptySet(); } - final UidPermissionState uidState = getUidState(ps, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + packageName + " and user " + userId); - return Collections.emptySet(); - } - if (!ps.getInstantApp(userId)) { - return uidState.getGrantedPermissions(); - } else { - // Install permission state is shared among all users, but instant app state is - // per-user, so we can only filter it here unless we make install permission state - // per-user as well. - final Set instantPermissions = new ArraySet<>(uidState.getGrantedPermissions()); - instantPermissions.removeIf(permissionName -> { - BasePermission permission = mSettings.getPermission(permissionName); - if (permission == null) { - return true; - } - if (!permission.isInstant()) { - EventLog.writeEvent(0x534e4554, "140256621", UserHandle.getUid(userId, - ps.getAppId()), permissionName); - return true; - } - return false; - }); - return instantPermissions; + + synchronized (mLock) { + final UidPermissionState uidState = getUidStateLocked(ps, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + packageName + " and user " + userId); + return Collections.emptySet(); + } + if (!ps.getInstantApp(userId)) { + return uidState.getGrantedPermissions(); + } else { + // Install permission state is shared among all users, but instant app state is + // per-user, so we can only filter it here unless we make install permission state + // per-user as well. + final Set instantPermissions = + new ArraySet<>(uidState.getGrantedPermissions()); + instantPermissions.removeIf(permissionName -> { + BasePermission permission = mSettings.getPermission(permissionName); + if (permission == null) { + return true; + } + if (!permission.isInstant()) { + EventLog.writeEvent(0x534e4554, "140256621", UserHandle.getUid(userId, + ps.getAppId()), permissionName); + return true; + } + return false; + }); + return instantPermissions; + } } } @@ -3546,13 +3575,15 @@ public class PermissionManagerService extends IPermissionManager.Stub { } // Legacy apps have the permission and get user consent on launch. - final UidPermissionState uidState = getUidState(pkg, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + " and user " - + userId); - return false; + synchronized (mLock) { + final UidPermissionState uidState = getUidStateLocked(pkg, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + " and user " + + userId); + return false; + } + return uidState.isPermissionReviewRequired(); } - return uidState.isPermissionReviewRequired(); } private void grantRequestedRuntimePermissions(AndroidPackage pkg, int[] userIds, @@ -3565,13 +3596,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { private void grantRequestedRuntimePermissionsForUser(AndroidPackage pkg, int userId, String[] grantedPermissions, int callingUid, PermissionCallback callback) { - final UidPermissionState uidState = getUidState(pkg, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + " and user " - + userId); - return; - } - final int immutableFlags = PackageManager.FLAG_PERMISSION_SYSTEM_FIXED | PackageManager.FLAG_PERMISSION_POLICY_FIXED; @@ -3593,7 +3617,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { && (supportsRuntimePermissions || !bp.isRuntimeOnly()) && (grantedPermissions == null || ArrayUtils.contains(grantedPermissions, permission))) { - final int flags = uidState.getPermissionFlags(permission); + final int flags = getPermissionFlagsInternal(permission, pkg.getPackageName(), + callingUid, userId); if (supportsRuntimePermissions) { // Installer cannot change immutable permissions. if ((flags & immutableFlags) == 0) { @@ -3621,12 +3646,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { for (int i = 0; i < userIds.length; i++) { int userId = userIds[i]; - final UidPermissionState uidState = getUidState(pkg, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + " and user " - + userId); - continue; - } for (int j = 0; j < permissionCount; j++) { final String permissionName = pkg.getRequestedPermissions().get(j); @@ -3637,14 +3656,26 @@ public class PermissionManagerService extends IPermissionManager.Stub { continue; } - if (uidState.isPermissionGranted(permissionName)) { + final boolean isGranted; + synchronized (mLock) { + final UidPermissionState uidState = getUidStateLocked(pkg, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + + " and user " + userId); + continue; + } + isGranted = uidState.isPermissionGranted(permissionName); + } + + if (isGranted) { if (oldGrantedRestrictedPermissions.get(userId) == null) { oldGrantedRestrictedPermissions.put(userId, new ArraySet<>()); } oldGrantedRestrictedPermissions.get(userId).add(permissionName); } - final int oldFlags = uidState.getPermissionFlags(permissionName); + final int oldFlags = getPermissionFlagsInternal(permissionName, + pkg.getPackageName(), callingUid, userId); int newFlags = oldFlags; int mask = 0; @@ -3708,7 +3739,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { // as whitelisting trumps policy i.e. policy cannot grant a non // grantable permission. if ((oldFlags & PackageManager.FLAG_PERMISSION_POLICY_FIXED) != 0) { - final boolean isGranted = uidState.isPermissionGranted(permissionName); if (!isWhitelisted && isGranted) { mask |= PackageManager.FLAG_PERMISSION_POLICY_FIXED; newFlags &= ~PackageManager.FLAG_PERMISSION_POLICY_FIXED; @@ -3742,15 +3772,19 @@ public class PermissionManagerService extends IPermissionManager.Stub { final int oldGrantedCount = oldPermsForUser.size(); for (int j = 0; j < oldGrantedCount; j++) { - final String permission = oldPermsForUser.valueAt(j); + final String permissionName = oldPermsForUser.valueAt(j); // Sometimes we create a new permission state instance during update. - final UidPermissionState newUidState = getUidState(pkg, userId); - if (newUidState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() - + " and user " + userId); - continue; + final boolean isGranted; + synchronized (mLock) { + final UidPermissionState uidState = getUidStateLocked(pkg, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName() + + " and user " + userId); + continue; + } + isGranted = uidState.isPermissionGranted(permissionName); } - if (!newUidState.isPermissionGranted(permission)) { + if (!isGranted) { callback.onPermissionRevoked(pkg.getUid(), userId, null); break; } @@ -3799,13 +3833,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { continue; } - UidPermissionState uidState = getUidState(deletedPs.pkg, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " + deletedPs.pkg.getPackageName() - + " and user " + userId); - continue; - } - PackageSetting disabledPs = mPackageManagerInt.getDisabledSystemPackage( deletedPs.pkg.getPackageName()); @@ -3824,10 +3851,19 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } - // TODO(zhanghai): Why are we only killing the UID when GIDs changed, instead of any - // permission change? - if (uidState.removePermissionState(bp.name) && bp.hasGids()) { - affectedUserId = userId; + synchronized (mLock) { + UidPermissionState uidState = getUidStateLocked(deletedPs.pkg, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + deletedPs.pkg.getPackageName() + + " and user " + userId); + continue; + } + + // TODO(zhanghai): Why are we only killing the UID when GIDs changed, instead of any + // permission change? + if (uidState.removePermissionState(bp.name) && bp.hasGids()) { + affectedUserId = userId; + } } } @@ -4122,14 +4158,17 @@ public class PermissionManagerService extends IPermissionManager.Stub { } else { mPackageManagerInt.forEachPackage(p -> { final int[] userIds = mUserManagerInt.getUserIds(); - for (final int userId : userIds) { - final UidPermissionState uidState = getUidState(p, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for " - + p.getPackageName() + " and user " + userId); - return; + synchronized (mLock) { + for (final int userId : userIds) { + final UidPermissionState uidState = getUidStateLocked(p, + userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for " + + p.getPackageName() + " and user " + userId); + continue; + } + uidState.removePermissionState(bp.name); } - uidState.removePermissionState(bp.name); } }); } @@ -4537,26 +4576,24 @@ public class PermissionManagerService extends IPermissionManager.Stub { } @Nullable - private UidPermissionState getUidState(@NonNull PackageSetting ps, + private UidPermissionState getUidStateLocked(@NonNull PackageSetting ps, @UserIdInt int userId) { - return getUidState(ps.getAppId(), userId); + return getUidStateLocked(ps.getAppId(), userId); } @Nullable - private UidPermissionState getUidState(@NonNull AndroidPackage pkg, + private UidPermissionState getUidStateLocked(@NonNull AndroidPackage pkg, @UserIdInt int userId) { - return getUidState(pkg.getUid(), userId); + return getUidStateLocked(pkg.getUid(), userId); } @Nullable - private UidPermissionState getUidState(int appId, @UserIdInt int userId) { - synchronized (mLock) { - final UserPermissionState userState = mState.getUserState(userId); - if (userState == null) { - return null; - } - return userState.getUidState(appId); + private UidPermissionState getUidStateLocked(@AppIdInt int appId, @UserIdInt int userId) { + final UserPermissionState userState = mState.getUserState(userId); + if (userState == null) { + return null; } + return userState.getUidState(appId); } private void removeAppIdState(@AppIdInt int appId) { @@ -4601,7 +4638,10 @@ public class PermissionManagerService extends IPermissionManager.Stub { } private void writeStateToPackageSettings() { - final int[] userIds = mState.getUserIds(); + final int[] userIds; + synchronized (mLock) { + userIds = mState.getUserIds(); + } mPackageManagerInt.forEachPackageSetting(ps -> { ps.setInstallPermissionsFixed(false); final LegacyPermissionState legacyState = ps.getLegacyPermissionState(); @@ -4652,27 +4692,30 @@ public class PermissionManagerService extends IPermissionManager.Stub { @NonNull private LegacyPermissionState getLegacyPermissionState(@AppIdInt int appId) { final LegacyPermissionState legacyState = new LegacyPermissionState(); - final int[] userIds = mState.getUserIds(); - for (final int userId : userIds) { - final UidPermissionState uidState = getUidState(appId, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for app ID " + appId + " and user ID " - + userId); - continue; - } - - final List permissionStates = uidState.getPermissionStates(); - final int permissionStatesSize = permissionStates.size(); - for (int i = 0; i < permissionStatesSize; i++) { - final PermissionState permissionState = permissionStates.get(i); + synchronized (mLock) { + final int[] userIds = mState.getUserIds(); + for (final int userId : userIds) { + final UidPermissionState uidState = getUidStateLocked(appId, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for app ID " + appId + " and user ID " + + userId); + continue; + } - final LegacyPermissionState.PermissionState legacyPermissionState = - new LegacyPermissionState.PermissionState(permissionState.getPermission(), - permissionState.isGranted(), permissionState.getFlags()); - if (permissionState.isRuntime()) { - legacyState.putRuntimePermissionState(legacyPermissionState, userId); - } else if (userId == UserHandle.USER_SYSTEM) { - legacyState.putInstallPermissionState(legacyPermissionState); + final List permissionStates = uidState.getPermissionStates(); + final int permissionStatesSize = permissionStates.size(); + for (int i = 0; i < permissionStatesSize; i++) { + final PermissionState permissionState = permissionStates.get(i); + + final LegacyPermissionState.PermissionState legacyPermissionState = + new LegacyPermissionState.PermissionState( + permissionState.getPermission(), permissionState.isGranted(), + permissionState.getFlags()); + if (permissionState.isRuntime()) { + legacyState.putRuntimePermissionState(legacyPermissionState, userId); + } else if (userId == UserHandle.USER_SYSTEM) { + legacyState.putInstallPermissionState(legacyPermissionState); + } } } } @@ -4683,12 +4726,15 @@ public class PermissionManagerService extends IPermissionManager.Stub { private int[] getGidsForUid(int uid) { final int appId = UserHandle.getAppId(uid); final int userId = UserHandle.getUserId(uid); - final UidPermissionState uidState = getUidState(appId, userId); - if (uidState == null) { - Slog.e(TAG, "Missing permissions state for app ID " + appId + " and user ID " + userId); - return EMPTY_INT_ARRAY; + synchronized (mLock) { + final UidPermissionState uidState = getUidStateLocked(appId, userId); + if (uidState == null) { + Slog.e(TAG, "Missing permissions state for app ID " + appId + " and user ID " + + userId); + return EMPTY_INT_ARRAY; + } + return uidState.computeGids(mGlobalGids, userId); } - return uidState.computeGids(mGlobalGids, userId); } private class PermissionManagerServiceInternalImpl extends PermissionManagerServiceInternal { diff --git a/services/core/java/com/android/server/pm/permission/UidPermissionState.java b/services/core/java/com/android/server/pm/permission/UidPermissionState.java index 82ab95464a28..38e8955e8cdb 100644 --- a/services/core/java/com/android/server/pm/permission/UidPermissionState.java +++ b/services/core/java/com/android/server/pm/permission/UidPermissionState.java @@ -24,8 +24,6 @@ import android.util.ArrayMap; import android.util.ArraySet; import android.util.IntArray; -import com.android.internal.annotations.GuardedBy; - import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -35,29 +33,23 @@ import java.util.Set; * Permission state for a UID. */ public final class UidPermissionState { - @NonNull - private final Object mLock = new Object(); - private boolean mMissing; - @GuardedBy("mLock") @Nullable private ArrayMap mPermissions; public UidPermissionState() {} public UidPermissionState(@NonNull UidPermissionState other) { - synchronized (mLock) { - mMissing = other.mMissing; + mMissing = other.mMissing; - if (other.mPermissions != null) { - mPermissions = new ArrayMap<>(); - final int permissionsSize = other.mPermissions.size(); - for (int i = 0; i < permissionsSize; i++) { - final String name = other.mPermissions.keyAt(i); - final PermissionState permissionState = other.mPermissions.valueAt(i); - mPermissions.put(name, new PermissionState(permissionState)); - } + if (other.mPermissions != null) { + mPermissions = new ArrayMap<>(); + final int permissionsSize = other.mPermissions.size(); + for (int i = 0; i < permissionsSize; i++) { + final String name = other.mPermissions.keyAt(i); + final PermissionState permissionState = other.mPermissions.valueAt(i); + mPermissions.put(name, new PermissionState(permissionState)); } } } @@ -66,11 +58,9 @@ public final class UidPermissionState { * Reset the internal state of this object. */ public void reset() { - synchronized (mLock) { - mMissing = false; - mPermissions = null; - invalidateCache(); - } + mMissing = false; + mPermissions = null; + invalidateCache(); } /** @@ -97,9 +87,7 @@ public final class UidPermissionState { */ @Deprecated public boolean hasPermissionState(@NonNull String name) { - synchronized (mLock) { - return mPermissions != null && mPermissions.containsKey(name); - } + return mPermissions != null && mPermissions.containsKey(name); } /** @@ -109,19 +97,17 @@ public final class UidPermissionState { */ @Deprecated public boolean hasPermissionState(@NonNull ArraySet names) { - synchronized (mLock) { - if (mPermissions == null) { - return false; - } - final int namesSize = names.size(); - for (int i = 0; i < namesSize; i++) { - final String name = names.valueAt(i); - if (mPermissions.containsKey(name)) { - return true; - } - } + if (mPermissions == null) { return false; } + final int namesSize = names.size(); + for (int i = 0; i < namesSize; i++) { + final String name = names.valueAt(i); + if (mPermissions.containsKey(name)) { + return true; + } + } + return false; } /** @@ -132,28 +118,24 @@ public final class UidPermissionState { */ @Nullable public PermissionState getPermissionState(@NonNull String name) { - synchronized (mLock) { - if (mPermissions == null) { - return null; - } - return mPermissions.get(name); + if (mPermissions == null) { + return null; } + return mPermissions.get(name); } @NonNull private PermissionState getOrCreatePermissionState(@NonNull BasePermission permission) { - synchronized (mLock) { - if (mPermissions == null) { - mPermissions = new ArrayMap<>(); - } - final String name = permission.getName(); - PermissionState permissionState = mPermissions.get(name); - if (permissionState == null) { - permissionState = new PermissionState(permission); - mPermissions.put(name, permissionState); - } - return permissionState; + if (mPermissions == null) { + mPermissions = new ArrayMap<>(); + } + final String name = permission.getName(); + PermissionState permissionState = mPermissions.get(name); + if (permissionState == null) { + permissionState = new PermissionState(permission); + mPermissions.put(name, permissionState); } + return permissionState; } /** @@ -163,12 +145,10 @@ public final class UidPermissionState { */ @NonNull public List getPermissionStates() { - synchronized (mLock) { - if (mPermissions == null) { - return Collections.emptyList(); - } - return new ArrayList<>(mPermissions.values()); + if (mPermissions == null) { + return Collections.emptyList(); } + return new ArrayList<>(mPermissions.values()); } /** @@ -179,20 +159,18 @@ public final class UidPermissionState { * @param flags the permission flags */ public void putPermissionState(@NonNull BasePermission permission, boolean granted, int flags) { - synchronized (mLock) { - final String name = permission.getName(); - if (mPermissions == null) { - mPermissions = new ArrayMap<>(); - } else { - mPermissions.remove(name); - } - final PermissionState permissionState = new PermissionState(permission); - if (granted) { - permissionState.grant(); - } - permissionState.updateFlags(flags, flags); - mPermissions.put(name, permissionState); + final String name = permission.getName(); + if (mPermissions == null) { + mPermissions = new ArrayMap<>(); + } else { + mPermissions.remove(name); } + final PermissionState permissionState = new PermissionState(permission); + if (granted) { + permissionState.grant(); + } + permissionState.updateFlags(flags, flags); + mPermissions.put(name, permissionState); } /** @@ -202,16 +180,14 @@ public final class UidPermissionState { * @return whether the permission state changed */ public boolean removePermissionState(@NonNull String name) { - synchronized (mLock) { - if (mPermissions == null) { - return false; - } - boolean changed = mPermissions.remove(name) != null; - if (changed && mPermissions.isEmpty()) { - mPermissions = null; - } - return changed; + if (mPermissions == null) { + return false; + } + final boolean changed = mPermissions.remove(name) != null; + if (changed && mPermissions.isEmpty()) { + mPermissions = null; } + return changed; } /** @@ -232,22 +208,20 @@ public final class UidPermissionState { */ @NonNull public Set getGrantedPermissions() { - synchronized (mLock) { - if (mPermissions == null) { - return Collections.emptySet(); - } + if (mPermissions == null) { + return Collections.emptySet(); + } - Set permissions = new ArraySet<>(mPermissions.size()); - final int permissionsSize = mPermissions.size(); - for (int i = 0; i < permissionsSize; i++) { - PermissionState permissionState = mPermissions.valueAt(i); + final Set permissions = new ArraySet<>(mPermissions.size()); + final int permissionsSize = mPermissions.size(); + for (int i = 0; i < permissionsSize; i++) { + final PermissionState permissionState = mPermissions.valueAt(i); - if (permissionState.isGranted()) { - permissions.add(permissionState.getName()); - } + if (permissionState.isGranted()) { + permissions.add(permissionState.getName()); } - return permissions; } + return permissions; } /** @@ -257,7 +231,7 @@ public final class UidPermissionState { * @return whether the permission grant state changed */ public boolean grantPermission(@NonNull BasePermission permission) { - PermissionState permissionState = getOrCreatePermissionState(permission); + final PermissionState permissionState = getOrCreatePermissionState(permission); return permissionState.grant(); } @@ -319,37 +293,33 @@ public final class UidPermissionState { if (flagMask == 0) { return false; } - synchronized (mLock) { - if (mPermissions == null) { - return false; - } - boolean anyChanged = false; - for (int i = mPermissions.size() - 1; i >= 0; i--) { - final PermissionState permissionState = mPermissions.valueAt(i); - final boolean changed = permissionState.updateFlags(flagMask, flagValues); - if (changed && permissionState.isDefault()) { - mPermissions.removeAt(i); - } - anyChanged |= changed; + if (mPermissions == null) { + return false; + } + boolean anyChanged = false; + for (int i = mPermissions.size() - 1; i >= 0; i--) { + final PermissionState permissionState = mPermissions.valueAt(i); + final boolean changed = permissionState.updateFlags(flagMask, flagValues); + if (changed && permissionState.isDefault()) { + mPermissions.removeAt(i); } - return anyChanged; + anyChanged |= changed; } + return anyChanged; } public boolean isPermissionReviewRequired() { - synchronized (mLock) { - if (mPermissions == null) { - return false; - } - final int permissionsSize = mPermissions.size(); - for (int i = 0; i < permissionsSize; i++) { - final PermissionState permission = mPermissions.valueAt(i); - if ((permission.getFlags() & PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED) != 0) { - return true; - } - } + if (mPermissions == null) { return false; } + final int permissionsSize = mPermissions.size(); + for (int i = 0; i < permissionsSize; i++) { + final PermissionState permission = mPermissions.valueAt(i); + if ((permission.getFlags() & PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED) != 0) { + return true; + } + } + return false; } /** @@ -360,24 +330,22 @@ public final class UidPermissionState { */ @NonNull public int[] computeGids(@NonNull int[] globalGids, @UserIdInt int userId) { - synchronized (mLock) { - IntArray gids = IntArray.wrap(globalGids); - if (mPermissions == null) { - return gids.toArray(); + IntArray gids = IntArray.wrap(globalGids); + if (mPermissions == null) { + return gids.toArray(); + } + final int permissionsSize = mPermissions.size(); + for (int i = 0; i < permissionsSize; i++) { + PermissionState permissionState = mPermissions.valueAt(i); + if (!permissionState.isGranted()) { + continue; } - final int permissionsSize = mPermissions.size(); - for (int i = 0; i < permissionsSize; i++) { - PermissionState permissionState = mPermissions.valueAt(i); - if (!permissionState.isGranted()) { - continue; - } - final int[] permissionGids = permissionState.computeGids(userId); - if (permissionGids.length != 0) { - gids.addAll(permissionGids); - } + final int[] permissionGids = permissionState.computeGids(userId); + if (permissionGids.length != 0) { + gids.addAll(permissionGids); } - return gids.toArray(); } + return gids.toArray(); } static void invalidateCache() { diff --git a/services/core/java/com/android/server/pm/permission/UserPermissionState.java b/services/core/java/com/android/server/pm/permission/UserPermissionState.java index 7f55cb161e40..2c741cfd6364 100644 --- a/services/core/java/com/android/server/pm/permission/UserPermissionState.java +++ b/services/core/java/com/android/server/pm/permission/UserPermissionState.java @@ -23,8 +23,6 @@ import android.os.UserHandle; import android.util.ArraySet; import android.util.SparseArray; -import com.android.internal.annotations.GuardedBy; - /** * Permission state for a user. */ @@ -33,71 +31,52 @@ public final class UserPermissionState { * Whether the install permissions have been granted to a package, so that no install * permissions should be added to it unless the package is upgraded. */ - @GuardedBy("mLock") @NonNull private final ArraySet mInstallPermissionsFixed = new ArraySet<>(); /** * Maps from app ID to {@link UidPermissionState}. */ - @GuardedBy("mLock") @NonNull private final SparseArray mUidStates = new SparseArray<>(); - @NonNull - private final Object mLock; - - public UserPermissionState(@NonNull Object lock) { - mLock = lock; - } - public boolean areInstallPermissionsFixed(@NonNull String packageName) { - synchronized (mLock) { - return mInstallPermissionsFixed.contains(packageName); - } + return mInstallPermissionsFixed.contains(packageName); } public void setInstallPermissionsFixed(@NonNull String packageName, boolean fixed) { - synchronized (mLock) { - if (fixed) { - mInstallPermissionsFixed.add(packageName); - } else { - mInstallPermissionsFixed.remove(packageName); - } + if (fixed) { + mInstallPermissionsFixed.add(packageName); + } else { + mInstallPermissionsFixed.remove(packageName); } } @Nullable public UidPermissionState getUidState(@AppIdInt int appId) { checkAppId(appId); - synchronized (mLock) { - return mUidStates.get(appId); - } + return mUidStates.get(appId); } @NonNull public UidPermissionState getOrCreateUidState(@AppIdInt int appId) { checkAppId(appId); - synchronized (mLock) { - UidPermissionState uidState = mUidStates.get(appId); - if (uidState == null) { - uidState = new UidPermissionState(); - mUidStates.put(appId, uidState); - } - return uidState; + UidPermissionState uidState = mUidStates.get(appId); + if (uidState == null) { + uidState = new UidPermissionState(); + mUidStates.put(appId, uidState); } + return uidState; } public void removeUidState(@AppIdInt int appId) { checkAppId(appId); - synchronized (mLock) { - mUidStates.delete(appId); - } + mUidStates.delete(appId); } private void checkAppId(@AppIdInt int appId) { if (UserHandle.getUserId(appId) != 0) { - throw new IllegalArgumentException(appId + " is not an app ID"); + throw new IllegalArgumentException("Invalid app ID " + appId); } } } -- cgit v1.2.3-59-g8ed1b