From 8846aaac9a4a1db819747f2eb24592e88d1d62b1 Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Thu, 13 Feb 2020 15:19:54 -0800 Subject: [PATCH] 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 --- .../android/app/AppOpsManagerInternal.java | 14 +++++----- .../android/server/appop/AppOpsService.java | 26 ++++++++++--------- .../policy/PermissionPolicyService.java | 6 ++--- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/core/java/android/app/AppOpsManagerInternal.java b/core/java/android/app/AppOpsManagerInternal.java index c13c5a5ab9c4d..309e91f1e4ffc 100644 --- a/core/java/android/app/AppOpsManagerInternal.java +++ b/core/java/android/app/AppOpsManagerInternal.java @@ -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); } diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index 3ffe1be890602..b5a24a8746ee7 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -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); - updatePermissionRevokedCompat(uid, code, mode); + 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 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); } } } diff --git a/services/core/java/com/android/server/policy/PermissionPolicyService.java b/services/core/java/com/android/server/policy/PermissionPolicyService.java index 43d45967f67bf..c13cb38544c5b 100644 --- a/services/core/java/com/android/server/policy/PermissionPolicyService.java +++ b/services/core/java/com/android/server/policy/PermissionPolicyService.java @@ -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); } }