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

Commit 8846aaac authored by Hai Zhang's avatar Hai Zhang
Browse files

Don't mutate permission state when setting app op from PermissionPolicyService. 

Previously the REVOKED_COMPAT flag may be updated when
PermissionPolicyService is setting app op mode, however it should not
do so because the app op mode is coming right from the permission
state it was seeing. Mutating permission state would lead to incorrect
permission state when PermissionPolicyService and PermissionController
are running concurrently, and by no longer mutating permission state
in PermissionPolicyService, the app op state will eventually arrive at
the correct state even when there's some concurrency.

Bug: 149267332
Bug: 136503238
Test: manual
Change-Id: I9b02bc3f42a20ba441d5af9fd1b8a9729d8d0480
parent 8eb3cbe1

core/java/android/app/AppOpsManagerInternal.java

+8 −6
Original line number Diff line number Diff line
@@ -94,14 +94,16 @@ public abstract class AppOpsManagerInternal { 
            boolean visible);



    /**

     * Like {@link AppOpsManager#setUidMode}, but allows ignoring a certain callback.

     * Like {@link AppOpsManager#setUidMode}, but allows ignoring our own callback and not updating

     * the REVOKED_COMPAT flag.

     */

    public abstract void setUidModeIgnoringCallback(int code, int uid, int mode,

            @Nullable IAppOpsCallback callbackToIgnore);

    public abstract void setUidModeFromPermissionPolicy(int code, int uid, int mode,

            @Nullable IAppOpsCallback callback);



    /**

     * Like {@link AppOpsManager#setMode}, but allows ignoring a certain callback.

     * Like {@link AppOpsManager#setMode}, but allows ignoring our own callback and not updating the

     * REVOKED_COMPAT flag.

     */

    public abstract void setModeIgnoringCallback(int code, int uid, @NonNull String packageName,

            int mode, @Nullable IAppOpsCallback callbackToIgnore);

    public abstract void setModeFromPermissionPolicy(int code, int uid, @NonNull String packageName,

            int mode, @Nullable IAppOpsCallback callback);

}

services/core/java/com/android/server/appop/AppOpsService.java

+14 −12
Original line number Diff line number Diff line
@@ -2006,7 +2006,7 @@ public class AppOpsService extends IAppOpsService.Stub { 
    }



    private void setUidMode(int code, int uid, int mode,

            @Nullable IAppOpsCallback callbackToIgnore) {

            @Nullable IAppOpsCallback permissionPolicyCallback) {

        if (DEBUG) {

            Slog.i(TAG, "uid " + uid + " OP_" + opToName(code) + " := " + modeToName(mode)

                    + " by uid " + Binder.getCallingUid());
@@ -2016,7 +2016,9 @@ public class AppOpsService extends IAppOpsService.Stub { 
        verifyIncomingOp(code);

        code = AppOpsManager.opToSwitch(code);



        if (permissionPolicyCallback == null) {

            updatePermissionRevokedCompat(uid, code, mode);

        }



        synchronized (this) {

            final int defaultMode = AppOpsManager.opToDefaultMode(code);
@@ -2054,7 +2056,7 @@ public class AppOpsService extends IAppOpsService.Stub { 
            uidState.evalForegroundOps(mOpModeWatchers);

        }



        notifyOpChangedForAllPkgsInUid(code, uid, false, callbackToIgnore);

        notifyOpChangedForAllPkgsInUid(code, uid, false, permissionPolicyCallback);

        notifyOpChangedSync(code, uid, null, mode);

    }
@@ -2256,7 +2258,7 @@ public class AppOpsService extends IAppOpsService.Stub { 
    }



    private void setMode(int code, int uid, @NonNull String packageName, int mode,

            @Nullable IAppOpsCallback callbackToIgnore) {

            @Nullable IAppOpsCallback permissionPolicyCallback) {

        enforceManageAppOpsModes(Binder.getCallingPid(), Binder.getCallingUid(), uid);

        verifyIncomingOp(code);

        ArraySet<ModeCallback> repCbs = null;
@@ -2300,8 +2302,8 @@ public class AppOpsService extends IAppOpsService.Stub { 
                        }

                        repCbs.addAll(cbs);

                    }

                    if (repCbs != null && callbackToIgnore != null) {

                        repCbs.remove(mModeWatchers.get(callbackToIgnore.asBinder()));

                    if (repCbs != null && permissionPolicyCallback != null) {

                        repCbs.remove(mModeWatchers.get(permissionPolicyCallback.asBinder()));

                    }

                    if (mode == AppOpsManager.opToDefaultMode(op.op)) {

                        // If going into the default mode, prune this op
@@ -5730,15 +5732,15 @@ public class AppOpsService extends IAppOpsService.Stub { 
        }



        @Override

        public void setUidModeIgnoringCallback(int code, int uid, int mode,

                @Nullable IAppOpsCallback callbackToIgnore) {

            setUidMode(code, uid, mode, callbackToIgnore);

        public void setUidModeFromPermissionPolicy(int code, int uid, int mode,

                @Nullable IAppOpsCallback callback) {

            setUidMode(code, uid, mode, callback);

        }



        @Override

        public void setModeIgnoringCallback(int code, int uid, @NonNull String packageName,

                int mode, @Nullable IAppOpsCallback callbackToIgnore) {

            setMode(code, uid, packageName, mode, callbackToIgnore);

        public void setModeFromPermissionPolicy(int code, int uid, @NonNull String packageName,

                int mode, @Nullable IAppOpsCallback callback) {

            setMode(code, uid, packageName, mode, callback);

        }

    }

}

services/core/java/com/android/server/policy/PermissionPolicyService.java

+3 −3
Original line number Diff line number Diff line
@@ -683,7 +683,7 @@ public final class PermissionPolicyService extends SystemService { 
                    opCode), uid, pkgsOfUid[0]);

            if (currentMode != MODE_ALLOWED) {

                if (currentMode != MODE_IGNORED) {

                    mAppOpsManagerInternal.setUidModeIgnoringCallback(opCode, uid, MODE_IGNORED,

                    mAppOpsManagerInternal.setUidModeFromPermissionPolicy(opCode, uid, MODE_IGNORED,

                            mAppOpsCallback);

                }

                return true;
@@ -700,7 +700,7 @@ public final class PermissionPolicyService extends SystemService { 
            final int oldMode = mAppOpsManager.unsafeCheckOpRaw(AppOpsManager.opToPublicName(

                    opCode), uid, pkgsOfUid[0]);

            if (oldMode != mode) {

                mAppOpsManagerInternal.setUidModeIgnoringCallback(opCode, uid, mode,

                mAppOpsManagerInternal.setUidModeFromPermissionPolicy(opCode, uid, mode,

                        mAppOpsCallback);

                final int newMode = mAppOpsManager.unsafeCheckOpRaw(AppOpsManager.opToPublicName(

                        opCode), uid, pkgsOfUid[0]);
@@ -708,7 +708,7 @@ public final class PermissionPolicyService extends SystemService { 
                    // Work around incorrectly-set package mode. It never makes sense for app ops

                    // related to runtime permissions, but can get in the way and we have to reset

                    // it.

                    mAppOpsManagerInternal.setModeIgnoringCallback(opCode, uid, pkgsOfUid[0],

                    mAppOpsManagerInternal.setModeFromPermissionPolicy(opCode, uid, pkgsOfUid[0],

                            AppOpsManager.opToDefaultMode(opCode), mAppOpsCallback);

                }

            }