Add onStateMutated() callback for listener dispatching.

This solves the issue of new state visibility when we invoke
externally provided listeners - they may check the current state when
they are called and they should always see the new state, but
currently they may see the old state if the thread mutating the state
hasn't finished mutation.

The onStateMutated() callback provides a way to invoke the externally
provided listener at the right time (after state mutation is
committed), so that they can always see the new state without worrying
about the race condition. Meanwhile, this also provides a good place
to consolidate the invocation for listeners like
OnPermissionsChangeListener which only needs to be invoked once for
each UID.

Changed the listeners to use abstract classes instead of interfaces
for slightly better performance in accordance with System Health
team's guidelines.

Made the OnPermissionsChangeListener only notified when runtime
permissions are changed. Previously we had the code to notify it when
permission flags for install permissions has changed, however the
PackageManager permission flags actually should only be applied to
runtime permissions. Meanwhile also existing grantRuntimePermission()
may grant a non-runtime permission like a development or role
permission, the existing implementation actually doesn't notify the
listener in this case, so notifying for flags change that doesn't
happen while not notifying for grant state changes makes even less
sense.

Bug: 252884423
Test: presubmit
Change-Id: I1f973e5e3c0ad4aa6ae23f6e238aef5aafa66e2e
This commit is contained in:
Hai Zhang
2022-12-16 04:24:53 -08:00
parent 46a23cd074
commit 60d184bdfa
8 changed files with 349 additions and 59 deletions

View File

@@ -161,6 +161,7 @@ class AccessCheckingService(context: Context) : SystemService(context) {
MutateStateScope(oldState, newState).action()
persistence.write(newState)
state = newState
with(policy) { GetStateScope(newState).onStateMutated() }
}
}

View File

@@ -73,6 +73,12 @@ class AccessPolicy private constructor(
}
}
fun GetStateScope.onStateMutated() {
forEachSchemePolicy {
with(it) { onStateMutated() }
}
}
fun MutateStateScope.onUserAdded(userId: Int) {
newState.systemState.userIds += userId
newState.userStates[userId] = UserState()
@@ -284,6 +290,8 @@ abstract class SchemePolicy {
decision: Int
)
open fun GetStateScope.onStateMutated() {}
open fun MutateStateScope.onUserAdded(userId: Int) {}
open fun MutateStateScope.onUserRemoved(userId: Int) {}

View File

@@ -48,6 +48,10 @@ class PackageAppOpPolicy : BaseAppOpPolicy(PackageAppOpPersistence()) {
setAppOpMode(subject.packageName, subject.userId, `object`.appOpName, decision)
}
override fun GetStateScope.onStateMutated() {
onAppOpModeChangedListeners.forEachIndexed { _, it -> it.onStateMutated() }
}
override fun MutateStateScope.onPackageRemoved(packageName: String, appId: Int) {
newState.userStates.forEachIndexed { _, _, userState ->
userState.packageAppOpModes -= packageName
@@ -113,13 +117,30 @@ class PackageAppOpPolicy : BaseAppOpPolicy(PackageAppOpPersistence()) {
}
}
fun interface OnAppOpModeChangedListener {
fun onAppOpModeChanged(
/**
* Listener for app op mode changes.
*/
abstract class OnAppOpModeChangedListener {
/**
* Called when an app op mode change has been made to the upcoming new state.
*
* Implementations should keep this method fast to avoid stalling the locked state mutation,
* and only call external code after [onStateMutated] when the new state has actually become
* the current state visible to external code.
*/
abstract fun onAppOpModeChanged(
packageName: String,
userId: Int,
appOpName: String,
oldMode: Int,
newMode: Int
)
/**
* Called when the upcoming new state has become the current state.
*
* Implementations should keep this method fast to avoid stalling the locked state mutation.
*/
abstract fun onStateMutated()
}
}

View File

@@ -48,6 +48,10 @@ class UidAppOpPolicy : BaseAppOpPolicy(UidAppOpPersistence()) {
setAppOpMode(subject.appId, subject.userId, `object`.appOpName, decision)
}
override fun GetStateScope.onStateMutated() {
onAppOpModeChangedListeners.forEachIndexed { _, it -> it.onStateMutated() }
}
override fun MutateStateScope.onAppIdRemoved(appId: Int) {
newState.userStates.forEachIndexed { _, _, userState ->
userState.uidAppOpModes -= appId
@@ -113,13 +117,30 @@ class UidAppOpPolicy : BaseAppOpPolicy(UidAppOpPersistence()) {
}
}
fun interface OnAppOpModeChangedListener {
fun onAppOpModeChanged(
/**
* Listener for app op mode changes.
*/
abstract class OnAppOpModeChangedListener {
/**
* Called when an app op mode change has been made to the upcoming new state.
*
* Implementations should keep this method fast to avoid stalling the locked state mutation,
* and only call external code after [onStateMutated] when the new state has actually become
* the current state visible to external code.
*/
abstract fun onAppOpModeChanged(
appId: Int,
userId: Int,
appOpName: String,
oldMode: Int,
newMode: Int
)
/**
* Called when the upcoming new state has become the current state.
*
* Implementations should keep this method fast to avoid stalling the locked state mutation.
*/
abstract fun onStateMutated()
}
}

View File

@@ -0,0 +1,171 @@
/*
* Copyright (C) 2022 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.server.permission.access.collection
import android.util.SparseBooleanArray
typealias IntBooleanMap = SparseBooleanArray
inline fun IntBooleanMap.allIndexed(predicate: (Int, Int, Boolean) -> Boolean): Boolean {
forEachIndexed { index, key, value ->
if (!predicate(index, key, value)) {
return false
}
}
return true
}
inline fun IntBooleanMap.anyIndexed(predicate: (Int, Int, Boolean) -> Boolean): Boolean {
forEachIndexed { index, key, value ->
if (predicate(index, key, value)) {
return true
}
}
return false
}
@Suppress("NOTHING_TO_INLINE")
inline fun IntBooleanMap.copy(): IntBooleanMap = clone()
inline fun <R> IntBooleanMap.firstNotNullOfOrNullIndexed(transform: (Int, Int, Boolean) -> R): R? {
forEachIndexed { index, key, value ->
transform(index, key, value)?.let { return it }
}
return null
}
inline fun IntBooleanMap.forEachIndexed(action: (Int, Int, Boolean) -> Unit) {
for (index in 0 until size) {
action(index, keyAt(index), valueAt(index))
}
}
inline fun IntBooleanMap.forEachKeyIndexed(action: (Int, Int) -> Unit) {
for (index in 0 until size) {
action(index, keyAt(index))
}
}
inline fun IntBooleanMap.forEachReversedIndexed(action: (Int, Int, Boolean) -> Unit) {
for (index in lastIndex downTo 0) {
action(index, keyAt(index), valueAt(index))
}
}
inline fun IntBooleanMap.forEachValueIndexed(action: (Int, Boolean) -> Unit) {
for (index in 0 until size) {
action(index, valueAt(index))
}
}
inline fun IntBooleanMap.getOrPut(key: Int, defaultValue: () -> Boolean): Boolean {
val index = indexOfKey(key)
return if (index >= 0) {
valueAt(index)
} else {
defaultValue().also { put(key, it) }
}
}
@Suppress("NOTHING_TO_INLINE")
inline fun IntBooleanMap?.getWithDefault(key: Int, defaultValue: Boolean): Boolean {
this ?: return defaultValue
return get(key, defaultValue)
}
inline val IntBooleanMap.lastIndex: Int
get() = size - 1
@Suppress("NOTHING_TO_INLINE")
inline operator fun IntBooleanMap.minusAssign(key: Int) {
delete(key)
}
inline fun IntBooleanMap.noneIndexed(predicate: (Int, Int, Boolean) -> Boolean): Boolean {
forEachIndexed { index, key, value ->
if (predicate(index, key, value)) {
return false
}
}
return true
}
@Suppress("NOTHING_TO_INLINE")
inline fun IntBooleanMap.putWithDefault(key: Int, value: Boolean, defaultValue: Boolean): Boolean {
val index = indexOfKey(key)
if (index >= 0) {
val oldValue = valueAt(index)
if (value != oldValue) {
if (value == defaultValue) {
removeAt(index)
} else {
setValueAt(index, value)
}
}
return oldValue
} else {
if (value != defaultValue) {
put(key, value)
}
return defaultValue
}
}
fun IntBooleanMap.remove(key: Int) {
delete(key)
}
fun IntBooleanMap.remove(key: Int, defaultValue: Boolean): Boolean {
val index = indexOfKey(key)
return if (index >= 0) {
val oldValue = valueAt(index)
removeAt(index)
oldValue
} else {
defaultValue
}
}
inline fun IntBooleanMap.removeAllIndexed(predicate: (Int, Int, Boolean) -> Boolean): Boolean {
var isChanged = false
forEachReversedIndexed { index, key, value ->
if (predicate(index, key, value)) {
removeAt(index)
isChanged = true
}
}
return isChanged
}
inline fun IntBooleanMap.retainAllIndexed(predicate: (Int, Int, Boolean) -> Boolean): Boolean {
var isChanged = false
forEachReversedIndexed { index, key, value ->
if (!predicate(index, key, value)) {
removeAt(index)
isChanged = true
}
}
return isChanged
}
@Suppress("NOTHING_TO_INLINE")
inline operator fun IntBooleanMap.set(key: Int, value: Boolean) {
put(key, value)
}
inline val IntBooleanMap.size: Int
get() = size()

View File

@@ -47,18 +47,18 @@ import com.android.server.LocalManagerRegistry
import com.android.server.LocalServices
import com.android.server.ServiceThread
import com.android.server.SystemConfig
import com.android.server.pm.PackageManagerLocal
import com.android.server.pm.permission.PermissionManagerServiceInterface
import com.android.server.permission.access.AccessCheckingService
import com.android.server.permission.access.PermissionUri
import com.android.server.permission.access.UidUri
import com.android.server.permission.access.collection.* // ktlint-disable no-wildcard-imports
import com.android.server.permission.access.util.hasAnyBit
import com.android.server.permission.access.util.hasBits
import com.android.server.pm.PackageManagerLocal
import com.android.server.pm.UserManagerService
import com.android.server.pm.permission.LegacyPermission
import com.android.server.pm.permission.LegacyPermissionSettings
import com.android.server.pm.permission.LegacyPermissionState
import com.android.server.pm.permission.PermissionManagerServiceInterface
import com.android.server.pm.permission.PermissionManagerServiceInternal
import com.android.server.pm.pkg.AndroidPackage
import java.io.FileDescriptor
@@ -85,7 +85,7 @@ class PermissionService(
private lateinit var handler: Handler
private lateinit var onPermissionsChangeListeners: OnPermissionsChangeListeners
private lateinit var permissionFlagsListener: OnPermissionFlagsChangedListener
private lateinit var onPermissionFlagsChangedListener: OnPermissionFlagsChangedListener
fun initialize() {
packageManagerInternal = LocalServices.getService(PackageManagerInternal::class.java)
@@ -100,8 +100,8 @@ class PermissionService(
handler = Handler(handlerThread.looper)
onPermissionsChangeListeners = OnPermissionsChangeListeners(FgThread.get().looper)
permissionFlagsListener = OnPermissionFlagsChangedListener()
policy.addOnPermissionFlagsChangedListener(permissionFlagsListener)
onPermissionFlagsChangedListener = OnPermissionFlagsChangedListener()
policy.addOnPermissionFlagsChangedListener(onPermissionFlagsChangedListener)
}
override fun getAllPermissionGroups(flags: Int): List<PermissionGroupInfo> {
@@ -234,9 +234,7 @@ class PermissionService(
with(policy) { getPermissionGroups()[permissionGroupName] }
} ?: return null
if (!snapshot.isPackageVisibleToUid(
permissionGroup.packageName, callingUid
)) {
if (!snapshot.isPackageVisibleToUid(permissionGroup.packageName, callingUid)) {
return null
}
}
@@ -674,9 +672,11 @@ class PermissionService(
* If you're doing surgery on app code/data, use [PackageFreezer] to guard your work against
* the app being relaunched.
*/
private fun killUid(appId: Int, userId: Int, reason: String) {
private fun killUid(uid: Int, reason: String) {
val activityManager = ActivityManager.getService()
if (activityManager != null) {
val appId = UserHandle.getAppId(uid)
val userId = UserHandle.getUserId(uid)
val identity = Binder.clearCallingIdentity()
try {
activityManager.killUidForPermissionChange(appId, userId, reason)
@@ -729,7 +729,12 @@ class PermissionService(
* Callback invoked when interesting actions have been taken on a permission.
*/
private inner class OnPermissionFlagsChangedListener :
UidPermissionPolicy.OnPermissionFlagsChangedListener {
UidPermissionPolicy.OnPermissionFlagsChangedListener() {
private val runtimePermissionChangedUids = IntSet()
// Mapping from UID to whether only notifications permissions are revoked.
private val runtimePermissionRevokedUids = IntBooleanMap()
private val gidsChangedUids = IntSet()
override fun onPermissionFlagsChanged(
appId: Int,
userId: Int,
@@ -741,37 +746,49 @@ class PermissionService(
val permission = service.getState {
with(policy) { getPermissions()[permissionName] }
} ?: return
val wasPermissionGranted = PermissionFlags.isPermissionGranted(oldFlags)
val isPermissionGranted = PermissionFlags.isPermissionGranted(newFlags)
val isPermissionGranted = !PermissionFlags.isPermissionGranted(oldFlags) &&
PermissionFlags.isPermissionGranted(newFlags)
val isPermissionRevoked = PermissionFlags.isPermissionGranted(oldFlags) &&
!PermissionFlags.isPermissionGranted(newFlags)
if (permission.isRuntime) {
// Different from the old implementation, which notifies the listeners when the
// permission flags have changed for a non-runtime permission, now we no longer do
// that because permission flags are only for runtime permissions and the listeners
// aren't being notified of non-runtime permission grant state changes anyway.
runtimePermissionChangedUids += uid
if (wasPermissionGranted && !isPermissionGranted) {
runtimePermissionRevokedUids[uid] =
permissionName in NOTIFICATIONS_PERMISSIONS &&
runtimePermissionRevokedUids.getWithDefault(uid, true)
}
}
if (isPermissionGranted) {
if (permission.isRuntime) {
onPermissionsChangeListeners.onPermissionsChanged(uid)
}
handler.post {
if (permission.hasGids) {
killUid(appId, userId, PermissionManager.KILL_APP_REASON_GIDS_CHANGED)
}
}
} else if (isPermissionRevoked) {
// TODO: STOPSHIP skip kill for revokePostNotificationPermissionWithoutKillForTest
if (permission.isRuntime) {
onPermissionsChangeListeners.onPermissionsChanged(uid)
handler.post {
if (!(permissionName == Manifest.permission.POST_NOTIFICATIONS &&
isAppBackupAndRestoreRunning(uid))) {
killUid(
appId, userId, PermissionManager.KILL_APP_REASON_PERMISSIONS_REVOKED
)
}
}
}
} else if (oldFlags != newFlags) {
if (permission.hasGids && !wasPermissionGranted && isPermissionGranted) {
gidsChangedUids += uid
}
}
override fun onStateMutated() {
runtimePermissionChangedUids.forEachIndexed { _, uid ->
onPermissionsChangeListeners.onPermissionsChanged(uid)
}
runtimePermissionChangedUids.clear()
runtimePermissionRevokedUids.forEachIndexed {
_, uid, areOnlyNotificationsPermissionsRevoked ->
handler.post {
if (areOnlyNotificationsPermissionsRevoked &&
isAppBackupAndRestoreRunning(uid)) {
return@post
}
killUid(uid, PermissionManager.KILL_APP_REASON_PERMISSIONS_REVOKED)
}
}
runtimePermissionRevokedUids.clear()
gidsChangedUids.forEachIndexed { _, uid ->
handler.post { killUid(uid, PermissionManager.KILL_APP_REASON_GIDS_CHANGED) }
}
gidsChangedUids.clear()
}
private fun isAppBackupAndRestoreRunning(uid: Int): Boolean {
@@ -780,13 +797,13 @@ class PermissionService(
return false
}
return try {
val contentResolver = context.contentResolver
val userId = UserHandle.getUserId(uid)
val isInSetup = Settings.Secure.getIntForUser(
context.contentResolver, Settings.Secure.USER_SETUP_COMPLETE, userId
contentResolver, Settings.Secure.USER_SETUP_COMPLETE, userId
) == 0
val isInDeferredSetup = Settings.Secure.getIntForUser(
context.contentResolver,
Settings.Secure.USER_SETUP_PERSONALIZATION_STATE, userId
contentResolver, Settings.Secure.USER_SETUP_PERSONALIZATION_STATE, userId
) == Settings.Secure.USER_SETUP_PERSONALIZATION_STARTED
isInSetup || isInDeferredSetup
} catch (e: Settings.SettingNotFoundException) {
@@ -809,18 +826,12 @@ class PermissionService(
}
private fun handleOnPermissionsChanged(uid: Int) {
val count = listeners.beginBroadcast()
try {
for (i in 0 until count) {
val callback = listeners.getBroadcastItem(i)
try {
callback.onPermissionsChanged(uid)
} catch (e: RemoteException) {
Log.e(LOG_TAG, "Permission listener is dead", e)
}
listeners.broadcast { listener ->
try {
listener.onPermissionsChanged(uid)
} catch (e: RemoteException) {
Log.e(LOG_TAG, "Error when calling OnPermissionsChangeListener", e)
}
} finally {
listeners.finishBroadcast()
}
}
@@ -837,6 +848,7 @@ class PermissionService(
obtainMessage(MSG_ON_PERMISSIONS_CHANGED, uid, 0).sendToTarget()
}
}
companion object {
private const val MSG_ON_PERMISSIONS_CHANGED = 1
}
@@ -852,5 +864,9 @@ class PermissionService(
@ChangeId
@EnabledAfter(targetSdkVersion = Build.VERSION_CODES.Q)
private val BACKGROUND_RATIONALE_CHANGE_ID = 147316723L
private val NOTIFICATIONS_PERMISSIONS = indexedSetOf(
Manifest.permission.POST_NOTIFICATIONS
)
}
}

View File

@@ -74,6 +74,10 @@ class UidPermissionPolicy : SchemePolicy() {
setPermissionFlags(subject.appId, subject.userId, `object`.permissionName, decision)
}
override fun GetStateScope.onStateMutated() {
onPermissionFlagsChangedListeners.forEachIndexed { _, it -> it.onStateMutated() }
}
override fun MutateStateScope.onUserAdded(userId: Int) {
newState.systemState.packageStates.forEach { (_, packageState) ->
evaluateAllPermissionStatesForPackageAndUser(packageState, userId, null)
@@ -563,7 +567,7 @@ class UidPermissionPolicy : SchemePolicy() {
newFlags = newFlags andInv PermissionFlags.LEGACY_GRANTED
val wasGrantedByImplicit = newFlags.hasBits(PermissionFlags.IMPLICIT_GRANTED)
val isLeanBackNotificationsPermission = newState.systemState.isLeanback &&
permissionName in NOTIFICATION_PERMISSIONS
permissionName in NOTIFICATIONS_PERMISSIONS
val isImplicitPermission = anyPackageInAppId(appId) {
permissionName in it.androidPackage!!.implicitPermissions
}
@@ -1108,18 +1112,35 @@ class UidPermissionPolicy : SchemePolicy() {
Manifest.permission.BLUETOOTH_SCAN
)
private val NOTIFICATION_PERMISSIONS = indexedSetOf(
private val NOTIFICATIONS_PERMISSIONS = indexedSetOf(
Manifest.permission.POST_NOTIFICATIONS
)
}
fun interface OnPermissionFlagsChangedListener {
fun onPermissionFlagsChanged(
/**
* Listener for permission flags changes.
*/
abstract class OnPermissionFlagsChangedListener {
/**
* Called when a permission flags change has been made to the upcoming new state.
*
* Implementations should keep this method fast to avoid stalling the locked state mutation,
* and only call external code after [onStateMutated] when the new state has actually become
* the current state visible to external code.
*/
abstract fun onPermissionFlagsChanged(
appId: Int,
userId: Int,
permissionName: String,
oldFlags: Int,
newFlags: Int
)
/**
* Called when the upcoming new state has become the current state.
*
* Implementations should keep this method fast to avoid stalling the locked state mutation.
*/
abstract fun onStateMutated()
}
}

View File

@@ -0,0 +1,31 @@
/*
* Copyright (C) 2022 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.server.permission.access.util
import android.os.IInterface
import android.os.RemoteCallbackList
inline fun <T : IInterface> RemoteCallbackList<T>.broadcast(action: (T) -> Unit) {
val itemCount = beginBroadcast()
try {
for (i in 0 until itemCount) {
action(getBroadcastItem(i))
}
} finally {
finishBroadcast()
}
}