Commit a7ed475d authored Dec 10, 2019 by Hai Zhang

Work around per-package app op mode set for runtime permission app op.

By resetting the per-package mode if it's still wrong after
setUidMode().

Runtime permissions and their app ops are controlled per-uid so it
never makes sense to have a per-package mode for it. However,
AppOpsService trims per-uid state if it's default (usually allowed),
and falls back to per-package state if per-uid state is not
found. This works fine when everybody behaves and don't touch the
per-package state for runtime permission app ops, so it is also left
in the default (allowed) state.

However, it seems that in Q someone (in the system or with
MANAGE_APP_OPS_MODES permission) used setMode() instead of
setUidMode() for OP_COARSE_LOCATION and set the per-package mode to
MODE_FOREGROUND. This means the affected app will never be able to get
background location access (because even when per-uid mode is set to
allowed by granting location permissions, the per-package mode is
consulted), and there's no way to recover from this in UI because
normally we never change per-package mode. And in R this resulted in
b/145744192 in which PermissionPolicyService wasn't able to sync the
app op mode correctly.

Bug: 145744192
Test: Flash without this workaround.
Test: adb shell appops set com.google.android.gms
      android:coarse_location foreground
Test: Check that the per-package mode for COARSE_LOCATION became
      foreground with adb shell appops get com.google.android.gms
Test: Check that PermissionPolicyService or LocationManagerService is
      looping with stacks system_server
Test: Flash with this workaroud.
Test: Check that the per-package mode for COARSE_LOCATION became
      allowed with adb shell appops get com.google.android.gms
Test: Check that PermissionPolicyService and LocationManagerService is
      not looping with stacks system_server

Change-Id: I2733ffad03a01fc54c961b9d5f382a3460458b8e

parent 5b0ee657

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

+12 −3

Original line number	Diff line number	Diff line
		@@ -676,10 +676,19 @@ public final class PermissionPolicyService extends SystemService {

		private void setUidMode(int opCode, int uid, int mode,
		@NonNull String packageName) {
		final int currentMode = mAppOpsManager.unsafeCheckOpRaw(AppOpsManager
		.opToPublicName(opCode), uid, packageName);
		if (currentMode != mode) {
		final int oldMode = mAppOpsManager.unsafeCheckOpRaw(AppOpsManager.opToPublicName(
		opCode), uid, packageName);
		if (oldMode != mode) {
		mAppOpsManager.setUidMode(opCode, uid, mode);
		final int newMode = mAppOpsManager.unsafeCheckOpRaw(AppOpsManager.opToPublicName(
		opCode), uid, packageName);
		if (newMode != mode) {
		// 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.
		mAppOpsManager.setMode(opCode, uid, packageName, AppOpsManager.opToDefaultMode(
		opCode));
		}
		}
		}