Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a71845ef authored by Hai Zhang's avatar Hai Zhang
Browse files

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
parent 063baa77

services/core/java/com/android/server/pm/permission/BasePermission.java

+0 −12
Original line number Diff line number Diff line
@@ -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<BasePermission> permissionTrees, String permName) {

        for (BasePermission bp : permissionTrees) {

services/core/java/com/android/server/pm/permission/DevicePermissionState.java

+13 −32
Original line number Diff line number Diff line
@@ -21,50 +21,32 @@ 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<UserPermissionState> 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);

    }

    }



    @NonNull

    public UserPermissionState getOrCreateUserState(@UserIdInt int userId) {

        synchronized (mLock) {

        UserPermissionState userState = mUserStates.get(userId);

        if (userState == null) {

                userState = new UserPermissionState(mLock);

            userState = new UserPermissionState();

            mUserStates.put(userId, userState);

        }

        return userState;

    }

    }



    public void removeUserState(@UserIdInt int userId) {

        synchronized (mLock) {

        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++) {
@@ -74,4 +56,3 @@ public final class DevicePermissionState { 
        return userIds;

    }

}
 
}

services/core/java/com/android/server/pm/permission/PermissionManagerService.java

+369 −323

File changed.

Preview size limit exceeded, changes collapsed.

services/core/java/com/android/server/pm/permission/UidPermissionState.java

+98 −130
Original line number Diff line number Diff line
@@ -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,19 +33,14 @@ 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<String, PermissionState> mPermissions;



    public UidPermissionState() {}



    public UidPermissionState(@NonNull UidPermissionState other) {

        synchronized (mLock) {

        mMissing = other.mMissing;



        if (other.mPermissions != null) {
@@ -60,18 +53,15 @@ public final class UidPermissionState { 
            }

        }

    }

    }



    /**

     * Reset the internal state of this object.

     */

    public void reset() {

        synchronized (mLock) {

        mMissing = false;

        mPermissions = null;

        invalidateCache();

    }

    }



    /**

     * Check whether the permissions state is missing for a user. This can happen if permission
@@ -97,10 +87,8 @@ public final class UidPermissionState { 
     */

    @Deprecated

    public boolean hasPermissionState(@NonNull String name) {

        synchronized (mLock) {

        return mPermissions != null && mPermissions.containsKey(name);

    }

    }



    /**

     * Get whether there is a permission state for any of the permissions.
@@ -109,7 +97,6 @@ public final class UidPermissionState { 
     */

    @Deprecated

    public boolean hasPermissionState(@NonNull ArraySet<String> names) {

        synchronized (mLock) {

        if (mPermissions == null) {

            return false;

        }
@@ -122,7 +109,6 @@ public final class UidPermissionState { 
        }

        return false;

    }

    }



    /**

     * Gets the state for a permission or null if none.
@@ -132,17 +118,14 @@ public final class UidPermissionState { 
     */

    @Nullable

    public PermissionState getPermissionState(@NonNull String name) {

        synchronized (mLock) {

        if (mPermissions == null) {

            return null;

        }

        return mPermissions.get(name);

    }

    }



    @NonNull

    private PermissionState getOrCreatePermissionState(@NonNull BasePermission permission) {

        synchronized (mLock) {

        if (mPermissions == null) {

            mPermissions = new ArrayMap<>();

        }
@@ -154,7 +137,6 @@ public final class UidPermissionState { 
        }

        return permissionState;

    }

    }



    /**

     * Get all permission states.
@@ -163,13 +145,11 @@ public final class UidPermissionState { 
     */

    @NonNull

    public List<PermissionState> getPermissionStates() {

        synchronized (mLock) {

        if (mPermissions == null) {

            return Collections.emptyList();

        }

        return new ArrayList<>(mPermissions.values());

    }

    }



    /**

     * Put a permission state.
@@ -179,7 +159,6 @@ 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<>();
@@ -193,7 +172,6 @@ public final class UidPermissionState { 
        permissionState.updateFlags(flags, flags);

        mPermissions.put(name, permissionState);

    }

    }



    /**

     * Remove a permission state.
@@ -202,17 +180,15 @@ 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;

        final boolean changed = mPermissions.remove(name) != null;

        if (changed && mPermissions.isEmpty()) {

            mPermissions = null;

        }

        return changed;

    }

    }



    /**

     * Get whether a permission is granted.
@@ -232,15 +208,14 @@ public final class UidPermissionState { 
     */

    @NonNull

    public Set<String> getGrantedPermissions() {

        synchronized (mLock) {

        if (mPermissions == null) {

            return Collections.emptySet();

        }



            Set<String> permissions = new ArraySet<>(mPermissions.size());

        final Set<String> permissions = new ArraySet<>(mPermissions.size());

        final int permissionsSize = mPermissions.size();

        for (int i = 0; i < permissionsSize; i++) {

                PermissionState permissionState = mPermissions.valueAt(i);

            final PermissionState permissionState = mPermissions.valueAt(i);



            if (permissionState.isGranted()) {

                permissions.add(permissionState.getName());
@@ -248,7 +223,6 @@ public final class UidPermissionState { 
        }

        return permissions;

    }

    }



    /**

     * Grant a permission.
@@ -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,7 +293,6 @@ public final class UidPermissionState { 
        if (flagMask == 0) {

            return false;

        }

        synchronized (mLock) {

        if (mPermissions == null) {

            return false;

        }
@@ -334,10 +307,8 @@ public final class UidPermissionState { 
        }

        return anyChanged;

    }

    }



    public boolean isPermissionReviewRequired() {

        synchronized (mLock) {

        if (mPermissions == null) {

            return false;

        }
@@ -350,7 +321,6 @@ public final class UidPermissionState { 
        }

        return false;

    }

    }



    /**

     * Compute the Linux GIDs from the permissions granted to a user.
@@ -360,7 +330,6 @@ 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();
@@ -378,7 +347,6 @@ public final class UidPermissionState { 
        }

        return gids.toArray();

    }

    }



    static void invalidateCache() {

        PackageManager.invalidatePackageInfoCache();

services/core/java/com/android/server/pm/permission/UserPermissionState.java

+13 −34
Original line number Diff line number Diff line
@@ -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,52 +31,36 @@ 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<String> mInstallPermissionsFixed = new ArraySet<>();



    /**

     * Maps from app ID to {@link UidPermissionState}.

     */

    @GuardedBy("mLock")

    @NonNull

    private final SparseArray<UidPermissionState> 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);

    }

    }



    public void setInstallPermissionsFixed(@NonNull String packageName, boolean fixed) {

        synchronized (mLock) {

        if (fixed) {

            mInstallPermissionsFixed.add(packageName);

        } else {

            mInstallPermissionsFixed.remove(packageName);

        }

    }

    }



    @Nullable

    public UidPermissionState getUidState(@AppIdInt int appId) {

        checkAppId(appId);

        synchronized (mLock) {

        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();
@@ -86,18 +68,15 @@ public final class UserPermissionState { 
        }

        return uidState;

    }

    }



    public void removeUidState(@AppIdInt int appId) {

        checkAppId(appId);

        synchronized (mLock) {

        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);

        }

    }

}