From e3a3105ce3b12ec4dc5eac77d9113940f294d244 Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Tue, 29 Sep 2020 13:37:17 -0700 Subject: [PATCH] Further refactor UidPermissionState and GIDs. The GIDs returned by the original permission state implementation in R actually was never unique, but simply all GIDs from granted permission concatenated together, and PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED was returned when the length of the GIDs changed. This is equivalent to simply checking whether the permission whose grant state changed has GIDs or not, and can greatly simplify the logic. PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED was only used in two places anyway. The original permission actually would never return PERMISSION_OPERATION_FAILURE as well because it checks hasPermission() beforehand and returns PERMISSION_OPERATION_SUCCESS if there's nothing to change in grant/revokePermission(). The name PERMISSION_OPERATION_FAILURE isn't a great name for unchanged anyway, so grant/revokePermission() is now changed to simply return a boolean for whether the permission state is changed. The cache for isPermissionReviewRequired() is removed because it's broken in subtle cases and iterating over an ArrayMap isn't a terrible trade-off anyway, in exchange for simpler code and correct behavior. Made removePermissionState() public so that code that actually wants to erase the state permission doesn't need to perform a revocation followed by updating all flags to 0. Non-null arrays are preferred in APIs, the same as non-null collections, so the GIDs-related APIs are updated to return non-null int arrays as well. EmptyArray.INT is used instead of null so there shouldn't be any performance penalty. Also ensured that the APIs are returning copies instead of the original array to guard against accidental mutation. Bug: 158736025 Test: presubmit Change-Id: I606210e18e5f8f87b8f8408fe476a72c2b7ed1c1 --- .../java/com/android/server/SystemConfig.java | 4 +- .../server/pm/permission/BasePermission.java | 19 +- .../permission/PermissionManagerService.java | 108 ++----- .../server/pm/permission/PermissionState.java | 3 +- .../pm/permission/UidPermissionState.java | 305 ++++++------------ 5 files changed, 149 insertions(+), 290 deletions(-) diff --git a/core/java/com/android/server/SystemConfig.java b/core/java/com/android/server/SystemConfig.java index 396a84ffcfb8b..ed663cfeb6136 100644 --- a/core/java/com/android/server/SystemConfig.java +++ b/core/java/com/android/server/SystemConfig.java @@ -46,6 +46,7 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.XmlUtils; import libcore.io.IoUtils; +import libcore.util.EmptyArray; import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; @@ -55,7 +56,6 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.FileReader; import java.io.IOException; -import java.util.Arrays; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -95,7 +95,7 @@ public class SystemConfig { private static final String VENDOR_SKU_PROPERTY = "ro.boot.product.vendor.sku"; // Group-ids that are given to all packages as read from etc/permissions/*.xml. - int[] mGlobalGids; + int[] mGlobalGids = EmptyArray.INT; // These are the built-in uid -> permission mappings that were read from the // system configuration files. 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 524f9acf15cd1..e92589bf518bb 100644 --- a/services/core/java/com/android/server/pm/permission/BasePermission.java +++ b/services/core/java/com/android/server/pm/permission/BasePermission.java @@ -36,13 +36,14 @@ import android.os.UserHandle; import android.util.Log; import android.util.Slog; -import com.android.internal.util.ArrayUtils; import com.android.server.pm.DumpState; import com.android.server.pm.PackageManagerService; import com.android.server.pm.PackageSettingBase; import com.android.server.pm.parsing.PackageInfoUtils; import com.android.server.pm.parsing.pkg.AndroidPackage; +import libcore.util.EmptyArray; + import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlSerializer; @@ -95,7 +96,8 @@ public final class BasePermission { int uid; /** Additional GIDs given to apps granted this permission */ - private int[] gids; + @NonNull + private int[] gids = EmptyArray.INT; /** * Flag indicating that {@link #gids} should be adjusted based on the @@ -132,7 +134,7 @@ public final class BasePermission { public int getUid() { return uid; } - public void setGids(int[] gids, boolean perUser) { + public void setGids(@NonNull int[] gids, boolean perUser) { this.gids = gids; this.perUser = perUser; } @@ -141,18 +143,20 @@ public final class BasePermission { } public boolean hasGids() { - return !ArrayUtils.isEmpty(gids); + return gids.length != 0; } + @NonNull public int[] computeGids(int userId) { if (perUser) { final int[] userGids = new int[gids.length]; for (int i = 0; i < gids.length; i++) { - userGids[i] = UserHandle.getUid(userId, gids[i]); + final int gid = gids[i]; + userGids[i] = UserHandle.getUid(userId, gid); } return userGids; } else { - return gids; + return gids.length != 0 ? gids.clone() : gids; } } @@ -286,7 +290,8 @@ public final class BasePermission { pendingPermissionInfo.packageName = newPackageName; } uid = 0; - setGids(null, false); + gids = EmptyArray.INT; + perUser = false; } public boolean addToTree(@ProtectionLevel int protectionLevel, 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 474ce7c32c5ae..08d82c8e0506a 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -53,7 +53,6 @@ import static com.android.server.pm.PackageManagerService.DEBUG_PACKAGE_SCANNING import static com.android.server.pm.PackageManagerService.DEBUG_PERMISSIONS; import static com.android.server.pm.PackageManagerService.DEBUG_REMOVE; import static com.android.server.pm.PackageManagerService.PLATFORM_PACKAGE_NAME; -import static com.android.server.pm.permission.UidPermissionState.PERMISSION_OPERATION_FAILURE; import static java.util.concurrent.TimeUnit.SECONDS; @@ -152,6 +151,8 @@ import com.android.server.pm.permission.PermissionManagerServiceInternal.Permiss import com.android.server.policy.PermissionPolicyInternal; import com.android.server.policy.SoftRestrictedPermissionPolicy; +import libcore.util.EmptyArray; + import java.io.FileDescriptor; import java.io.PrintWriter; import java.lang.annotation.Retention; @@ -245,6 +246,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { private final SparseArray> mSystemPermissions; /** Built-in group IDs given to all packages. Read from system configuration files. */ + @NonNull private final int[] mGlobalGids; private final HandlerThread mHandlerThread; @@ -1501,7 +1503,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { // 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) != PERMISSION_OPERATION_FAILURE) { + if (uidState.grantPermission(bp)) { if (callback != null) { callback.onInstallPermissionGranted(); } @@ -1519,18 +1521,14 @@ public class PermissionManagerService extends IPermissionManager.Stub { return; } - final int result = uidState.grantPermission(bp); - switch (result) { - case PERMISSION_OPERATION_FAILURE: { - return; - } + if (!uidState.grantPermission(bp)) { + return; + } - case UidPermissionState.PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED: { - if (callback != null) { - callback.onGidsChanged(UserHandle.getAppId(pkg.getUid()), userId); - } + if (bp.hasGids()) { + if (callback != null) { + callback.onGidsChanged(UserHandle.getAppId(pkg.getUid()), userId); } - break; } if (bp.isRuntime()) { @@ -1650,7 +1648,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { // 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) != PERMISSION_OPERATION_FAILURE) { + if (uidState.revokePermission(bp)) { if (callback != null) { mDefaultPermissionCallback.onInstallPermissionRevoked(); } @@ -1658,12 +1656,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { return; } - // Permission is already revoked, no need to do anything. - if (!uidState.isPermissionGranted(permName)) { - return; - } - - if (uidState.revokePermission(bp) == PERMISSION_OPERATION_FAILURE) { + if (!uidState.revokePermission(bp)) { return; } @@ -2504,11 +2497,11 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } - @Nullable + @NonNull private int[] getPermissionGids(@NonNull String permissionName, @UserIdInt int userId) { BasePermission permission = mSettings.getPermission(permissionName); if (permission == null) { - return null; + return EmptyArray.INT; } return permission.computeGids(userId); } @@ -2629,8 +2622,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } - uidState.setGlobalGids(mGlobalGids); - ArraySet newImplicitPermissions = new ArraySet<>(); final String friendlyName = pkg.getPackageName() + "(" + pkg.getUid() + ")"; @@ -2765,7 +2756,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { switch (grant) { case GRANT_INSTALL: { // Grant an install permission. - if (uidState.grantPermission(bp) != PERMISSION_OPERATION_FAILURE) { + if (uidState.grantPermission(bp)) { changedInstallPermission = true; } } break; @@ -2797,8 +2788,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { if (permissionPolicyInitialized && hardRestricted) { if (!restrictionExempt) { if (origPermState != null && origPermState.isGranted() - && uidState.revokePermission( - bp) != PERMISSION_OPERATION_FAILURE) { + && uidState.revokePermission(bp)) { wasChanged = true; } if (!restrictionApplied) { @@ -2830,8 +2820,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { || (!hardRestricted || restrictionExempt)) { if ((origPermState != null && origPermState.isGranted()) || upgradedActivityRecognitionPermission != null) { - if (uidState.grantPermission(bp) - == PERMISSION_OPERATION_FAILURE) { + if (!uidState.grantPermission(bp)) { wasChanged = true; } } @@ -2850,8 +2839,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { } if (!uidState.isPermissionGranted(bp.name) - && uidState.grantPermission(bp) - != PERMISSION_OPERATION_FAILURE) { + && uidState.grantPermission(bp)) { wasChanged = true; } @@ -2899,13 +2887,11 @@ public class PermissionManagerService extends IPermissionManager.Stub { } break; } } else { - if (uidState.revokePermission(bp) != PERMISSION_OPERATION_FAILURE) { - // Also drop the permission flags. - uidState.updatePermissionFlags(bp, - MASK_PERMISSION_FLAGS_ALL, 0); - changedInstallPermission = true; - if (DEBUG_PERMISSIONS) { - Slog.i(TAG, "Un-granting permission " + perm + if (DEBUG_PERMISSIONS) { + boolean wasGranted = uidState.isPermissionGranted(bp.name); + if (wasGranted || bp.isAppOp()) { + Slog.i(TAG, (wasGranted ? "Un-granting" : "Not granting") + + " permission " + perm + " from package " + friendlyName + " (protectionLevel=" + bp.getProtectionLevel() + " flags=0x" @@ -2913,20 +2899,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { ps)) + ")"); } - } else if (bp.isAppOp()) { - // Don't print warning for app op permissions, since it is fine for them - // not to be granted, there is a UI for the user to decide. - if (DEBUG_PERMISSIONS - && (packageOfInterest == null - || packageOfInterest.equals(pkg.getPackageName()))) { - Slog.i(TAG, "Not granting permission " + perm - + " to package " + friendlyName - + " (protectionLevel=" + bp.getProtectionLevel() - + " flags=0x" - + Integer.toHexString(PackageInfoUtils.appInfoFlags(pkg, - ps)) - + ")"); - } + } + if (uidState.removePermissionState(bp.name)) { + changedInstallPermission = true; } } } @@ -3005,8 +2980,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { if ((flags & BLOCKING_PERMISSION_FLAGS) == 0 && supportsRuntimePermissions) { - int revokeResult = ps.revokePermission(bp); - if (revokeResult != PERMISSION_OPERATION_FAILURE) { + if (ps.revokePermission(bp)) { if (DEBUG_PERMISSIONS) { Slog.i(TAG, "Revoking runtime permission " + permission + " for " + pkgName @@ -3865,14 +3839,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } - // The package is gone - no need to keep flags for applying policy. - uidState.updatePermissionFlags(bp, PackageManager.MASK_PERMISSION_FLAGS_ALL, 0); - - // Try to revoke as a runtime permission which is per user. - // TODO(zhanghai): This doesn't make sense. revokePermission() doesn't fail, and why are - // we only killing the uid when gids changed, instead of any permission change? - if (uidState.revokePermission(bp) - == UidPermissionState.PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED) { + // 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; } } @@ -3905,17 +3874,14 @@ public class PermissionManagerService extends IPermissionManager.Stub { boolean runtimePermissionChanged = false; // Prune permissions - final List permissionStates = - uidState.getPermissionStates(); + final List permissionStates = uidState.getPermissionStates(); final int permissionStatesSize = permissionStates.size(); for (int i = permissionStatesSize - 1; i >= 0; i--) { PermissionState permissionState = permissionStates.get(i); if (!usedPermissions.contains(permissionState.getName())) { BasePermission bp = mSettings.getPermissionLocked(permissionState.getName()); if (bp != null) { - uidState.revokePermission(bp); - uidState.updatePermissionFlags(bp, MASK_PERMISSION_FLAGS_ALL, 0); - if (permissionState.isRuntime()) { + if (uidState.removePermissionState(bp.name) && permissionState.isRuntime()) { runtimePermissionChanged = true; } } @@ -4178,11 +4144,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { + p.getPackageName() + " and user " + userId); return; } - if (uidState.getPermissionState(bp.getName()) != null) { - uidState.revokePermission(bp); - uidState.updatePermissionFlags(bp, MASK_PERMISSION_FLAGS_ALL, - 0); - } + uidState.removePermissionState(bp.name); } }); } @@ -4741,7 +4703,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { Slog.e(TAG, "Missing permissions state for app ID " + appId + " and user ID " + userId); return EMPTY_INT_ARRAY; } - return uidState.computeGids(userId); + return uidState.computeGids(mGlobalGids, userId); } private class PermissionManagerServiceInternalImpl extends PermissionManagerServiceInternal { @@ -4804,7 +4766,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { @UserIdInt int userId) { return PermissionManagerService.this.getGrantedPermissions(packageName, userId); } - @Nullable + @NonNull @Override public int[] getPermissionGids(@NonNull String permissionName, @UserIdInt int userId) { return PermissionManagerService.this.getPermissionGids(permissionName, userId); diff --git a/services/core/java/com/android/server/pm/permission/PermissionState.java b/services/core/java/com/android/server/pm/permission/PermissionState.java index 38264c83a15c1..59b204f7dfff2 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionState.java +++ b/services/core/java/com/android/server/pm/permission/PermissionState.java @@ -17,7 +17,6 @@ package com.android.server.pm.permission; import android.annotation.NonNull; -import android.annotation.Nullable; import android.annotation.UserIdInt; import com.android.internal.annotations.GuardedBy; @@ -62,7 +61,7 @@ public final class PermissionState { return mPermission.getName(); } - @Nullable + @NonNull public int[] computeGids(@UserIdInt int userId) { return mPermission.computeGids(userId); } 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 06a7f8dbd2be7..c73e2f3e153b7 100644 --- a/services/core/java/com/android/server/pm/permission/UidPermissionState.java +++ b/services/core/java/com/android/server/pm/permission/UidPermissionState.java @@ -22,35 +22,19 @@ import android.annotation.UserIdInt; import android.content.pm.PackageManager; import android.util.ArrayMap; import android.util.ArraySet; +import android.util.IntArray; import com.android.internal.annotations.GuardedBy; -import com.android.internal.util.ArrayUtils; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Set; /** * Permission state for a UID. - *

- * This class is also responsible for keeping track of the Linux GIDs per - * user for a package or a shared user. The GIDs are computed as a set of - * the GIDs for all granted permissions' GIDs on a per user basis. */ public final class UidPermissionState { - /** The permission operation failed. */ - public static final int PERMISSION_OPERATION_FAILURE = -1; - - /** The permission operation succeeded and no gids changed. */ - public static final int PERMISSION_OPERATION_SUCCESS = 0; - - /** The permission operation succeeded and gids changed. */ - public static final int PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED = 1; - - private static final int[] NO_GIDS = {}; - @NonNull private final Object mLock = new Object(); @@ -60,11 +44,6 @@ public final class UidPermissionState { @Nullable private ArrayMap mPermissions; - private boolean mPermissionReviewRequired; - - @NonNull - private int[] mGlobalGids = NO_GIDS; - public UidPermissionState() {} public UidPermissionState(@NonNull UidPermissionState other) { @@ -80,12 +59,6 @@ public final class UidPermissionState { mPermissions.put(name, new PermissionState(permissionState)); } } - - mPermissionReviewRequired = other.mPermissionReviewRequired; - - if (other.mGlobalGids != NO_GIDS) { - mGlobalGids = other.mGlobalGids.clone(); - } } } @@ -96,8 +69,6 @@ public final class UidPermissionState { synchronized (mLock) { mMissing = false; mPermissions = null; - mPermissionReviewRequired = false; - mGlobalGids = NO_GIDS; invalidateCache(); } } @@ -156,8 +127,8 @@ public final class UidPermissionState { /** * Gets the state for a permission or null if none. * - * @param name the permission name. - * @return the permission state. + * @param name the permission name + * @return the permission state */ @Nullable public PermissionState getPermissionState(@NonNull String name) { @@ -169,6 +140,22 @@ public final class UidPermissionState { } } + @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; + } + } + /** * Get all permission states. * @@ -186,19 +173,44 @@ public final class UidPermissionState { /** * Put a permission state. + * + * @param permission the permission + * @param granted whether the permission is granted + * @param flags the permission flags */ - public void putPermissionState(@NonNull BasePermission permission, boolean isGranted, - int flags) { + public void putPermissionState(@NonNull BasePermission permission, boolean granted, int flags) { synchronized (mLock) { - ensureNoPermissionState(permission.name); - PermissionState permissionState = ensurePermissionState(permission); - if (isGranted) { + 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); - if ((flags & PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED) != 0) { - mPermissionReviewRequired = true; + mPermissions.put(name, permissionState); + } + } + + /** + * Remove a permission state. + * + * @param name the permission name + * @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; } } @@ -209,13 +221,8 @@ public final class UidPermissionState { * @return whether the permission is granted */ public boolean isPermissionGranted(@NonNull String name) { - synchronized (mLock) { - if (mPermissions == null) { - return false; - } - PermissionState permissionState = mPermissions.get(name); - return permissionState != null && permissionState.isGranted(); - } + final PermissionState permissionState = getPermissionState(name); + return permissionState != null && permissionState.isGranted(); } /** @@ -246,61 +253,37 @@ public final class UidPermissionState { /** * Grant a permission. * - * @param permission the permission to grantt - * @return the operation result, which is either {@link #PERMISSION_OPERATION_SUCCESS}, - * or {@link #PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED}, or {@link - * #PERMISSION_OPERATION_FAILURE}. + * @param permission the permission to grant + * @return whether the permission grant state changed */ - public int grantPermission(@NonNull BasePermission permission) { - if (isPermissionGranted(permission.getName())) { - return PERMISSION_OPERATION_SUCCESS; - } - - PermissionState permissionState = ensurePermissionState(permission); - - if (!permissionState.grant()) { - return PERMISSION_OPERATION_FAILURE; - } - - return permission.hasGids() ? PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED - : PERMISSION_OPERATION_SUCCESS; + public boolean grantPermission(@NonNull BasePermission permission) { + PermissionState permissionState = getOrCreatePermissionState(permission); + return permissionState.grant(); } /** * Revoke a permission. * * @param permission the permission to revoke - * @return the operation result, which is either {@link #PERMISSION_OPERATION_SUCCESS}, - * or {@link #PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED}, or {@link - * #PERMISSION_OPERATION_FAILURE}. + * @return whether the permission grant state changed */ - public int revokePermission(@NonNull BasePermission permission) { + public boolean revokePermission(@NonNull BasePermission permission) { final String name = permission.getName(); - if (!isPermissionGranted(name)) { - return PERMISSION_OPERATION_SUCCESS; + final PermissionState permissionState = getPermissionState(name); + if (permissionState == null) { + return false; } - - PermissionState permissionState; - synchronized (mLock) { - permissionState = mPermissions.get(name); + final boolean changed = permissionState.revoke(); + if (changed && permissionState.isDefault()) { + removePermissionState(name); } - - if (!permissionState.revoke()) { - return PERMISSION_OPERATION_FAILURE; - } - - if (permissionState.isDefault()) { - ensureNoPermissionState(name); - } - - return permission.hasGids() ? PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED - : PERMISSION_OPERATION_SUCCESS; + return changed; } /** * Get the flags for a permission. * - * @param name the permission name. + * @param name the permission name * @return the permission flags */ public int getPermissionFlags(@NonNull String name) { @@ -324,76 +307,36 @@ public final class UidPermissionState { if (flagMask == 0) { return false; } - - synchronized (mLock) { - final PermissionState permissionState = ensurePermissionState(permission); - final int oldFlags = permissionState.getFlags(); - - final boolean updated = permissionState.updateFlags(flagMask, flagValues); - if (updated) { - final int newFlags = permissionState.getFlags(); - if ((oldFlags & PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED) == 0 - && (newFlags & PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED) != 0) { - mPermissionReviewRequired = true; - } else if ((oldFlags & PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED) != 0 - && (newFlags & PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED) == 0) { - if (mPermissionReviewRequired && !hasPermissionRequiringReview()) { - mPermissionReviewRequired = false; - } - } - } - return updated; + final PermissionState permissionState = getOrCreatePermissionState(permission); + final boolean changed = permissionState.updateFlags(flagMask, flagValues); + if (changed && permissionState.isDefault()) { + removePermissionState(permission.name); } + return changed; } public boolean updatePermissionFlagsForAllPermissions(int flagMask, int flagValues) { + if (flagMask == 0) { + return false; + } synchronized (mLock) { if (mPermissions == null) { return false; } - boolean changed = false; - final int permissionsSize = mPermissions.size(); - for (int i = 0; i < permissionsSize; i++) { + boolean anyChanged = false; + for (int i = mPermissions.size() - 1; i >= 0; i--) { final PermissionState permissionState = mPermissions.valueAt(i); - changed |= permissionState.updateFlags(flagMask, flagValues); - } - return changed; - } - } - - @NonNull - private PermissionState ensurePermissionState(@NonNull BasePermission permission) { - final String name = permission.getName(); - synchronized (mLock) { - if (mPermissions == null) { - mPermissions = new ArrayMap<>(); - } - PermissionState permissionState = mPermissions.get(name); - if (permissionState == null) { - permissionState = new PermissionState(permission); - mPermissions.put(name, permissionState); - } - return permissionState; - } - } - - private void ensureNoPermissionState(@NonNull String name) { - synchronized (mLock) { - if (mPermissions == null) { - return; - } - mPermissions.remove(name); - if (mPermissions.isEmpty()) { - mPermissions = null; + final boolean changed = permissionState.updateFlags(flagMask, flagValues); + if (changed && permissionState.isDefault()) { + mPermissions.removeAt(i); + } + anyChanged |= changed; } + return anyChanged; } } public boolean isPermissionReviewRequired() { - return mPermissionReviewRequired; - } - - private boolean hasPermissionRequiringReview() { synchronized (mLock) { final int permissionsSize = mPermissions.size(); for (int i = 0; i < permissionsSize; i++) { @@ -406,27 +349,6 @@ public final class UidPermissionState { } } - /** - * Gets the global gids, applicable to all users. - */ - @NonNull - public int[] getGlobalGids() { - return mGlobalGids; - } - - /** - * Sets the global gids, applicable to all users. - * - * @param globalGids The global gids. - */ - public void setGlobalGids(@NonNull int[] globalGids) { - if (!ArrayUtils.isEmpty(globalGids)) { - mGlobalGids = Arrays.copyOf(globalGids, globalGids.length); - } else { - mGlobalGids = NO_GIDS; - } - } - /** * Compute the Linux GIDs from the permissions granted to a user. * @@ -434,54 +356,25 @@ public final class UidPermissionState { * @return the GIDs for the user */ @NonNull - public int[] computeGids(@UserIdInt int userId) { - int[] gids = mGlobalGids; - + public int[] computeGids(@NonNull int[] globalGids, @UserIdInt int userId) { synchronized (mLock) { - if (mPermissions != null) { - final int permissionCount = mPermissions.size(); - for (int i = 0; i < permissionCount; i++) { - PermissionState permissionState = mPermissions.valueAt(i); - if (!permissionState.isGranted()) { - continue; - } - final int[] permGids = permissionState.computeGids(userId); - if (permGids != NO_GIDS) { - gids = appendInts(gids, permGids); - } + 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[] permissionGids = permissionState.computeGids(userId); + if (permissionGids.length != 0) { + gids.addAll(permissionGids); } } + return gids.toArray(); } - - return gids; - } - - /** - * Compute the Linux GIDs from the permissions granted to specified users. - * - * @param userIds the user IDs - * @return the GIDs for the user - */ - @NonNull - public int[] computeGids(@NonNull int[] userIds) { - int[] gids = mGlobalGids; - - for (final int userId : userIds) { - final int[] userGids = computeGids(userId); - gids = appendInts(gids, userGids); - } - - return gids; - } - - // TODO: fix this to use arraycopy and append all ints in one go - private static int[] appendInts(int[] current, int[] added) { - if (current != null && added != null) { - for (int guid : added) { - current = ArrayUtils.appendInt(current, guid); - } - } - return current; } static void invalidateCache() {