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

Commit b2cf30ad authored by Yi-an Chen's avatar Yi-an Chen
Browse files

Correct the scan order in onStoreageVolumeAdded 

In this bug we discovered that the iteration order of
onStorageVolumeAdded doesn't follow the order when these packages are
scanned. In this CL we record the order when onPackageAdded are called
and use this order to iterate within onStorageVolumeAdded

Bug: 282984463
Test: presubmit
Change-Id: Id8225d6a16846a7f0f19c24c4adf9de5d9b06537
parent 3d3d7ab6

services/permission/java/com/android/server/permission/access/AccessCheckingService.kt

+6 −2
Original line number Diff line number Diff line
@@ -153,14 +153,18 @@ class AccessCheckingService(context: Context) : SystemService(context) { 
        }

    }



    internal fun onStorageVolumeMounted(volumeUuid: String?, isSystemUpdated: Boolean) {

    internal fun onStorageVolumeMounted(

        volumeUuid: String?,

        packageNames: List<String>,

        isSystemUpdated: Boolean

    ) {

        val (packageStates, disabledSystemPackageStates) = packageManagerLocal.allPackageStates

        val knownPackages = packageManagerInternal.getKnownPackages(packageStates)

        mutateState {

            with(policy) {

                onStorageVolumeMounted(

                    packageStates, disabledSystemPackageStates, knownPackages, volumeUuid,

                    isSystemUpdated

                    packageNames, isSystemUpdated

                )

            }

        }

services/permission/java/com/android/server/permission/access/AccessPolicy.kt

+8 −2
Original line number Diff line number Diff line
@@ -132,6 +132,7 @@ class AccessPolicy private constructor( 
        disabledSystemPackageStates: Map<String, PackageState>,

        knownPackages: IntMap<Array<String>>,

        volumeUuid: String?,

        packageNames: List<String>,

        isSystemUpdated: Boolean

    ) {

        val addedAppIds = MutableIntSet()
@@ -140,6 +141,10 @@ class AccessPolicy private constructor( 
            setDisabledSystemPackageStates(disabledSystemPackageStates)

            packageStates.forEach { (packageName, packageState) ->

                if (packageState.volumeUuid == volumeUuid) {

                    check(packageNames.contains(packageName)) {

                        "Package $packageName on storage volume $volumeUuid didn't receive" +

                            " onPackageAdded() before onStorageVolumeMounted()"

                    }

                    val appId = packageState.appId

                    mutateAppIdPackageNames().mutateOrPut(appId) {

                        addedAppIds += appId
@@ -155,7 +160,7 @@ class AccessPolicy private constructor( 
            }

        }

        forEachSchemePolicy {

            with(it) { onStorageVolumeMounted(volumeUuid, isSystemUpdated) }

            with(it) { onStorageVolumeMounted(volumeUuid, packageNames, isSystemUpdated) }

        }

        packageStates.forEach { (_, packageState) ->

            if (packageState.volumeUuid == volumeUuid) {
@@ -481,7 +486,8 @@ abstract class SchemePolicy { 


    open fun MutateStateScope.onStorageVolumeMounted(

        volumeUuid: String?,

        isSystemUpdated: Boolean

        packageNames: List<String>,

        isSystemUpdated: Boolean,

    ) {}



    open fun MutateStateScope.onPackageAdded(packageState: PackageState) {}

services/permission/java/com/android/server/permission/access/permission/AppIdPermissionPolicy.kt

+3 −5
Original line number Diff line number Diff line
@@ -138,14 +138,12 @@ class AppIdPermissionPolicy : SchemePolicy() { 


    override fun MutateStateScope.onStorageVolumeMounted(

        volumeUuid: String?,

        packageNames: List<String>,

        isSystemUpdated: Boolean

    ) {

        val changedPermissionNames = MutableIndexedSet<String>()

        newState.externalState.packageStates.forEach { (_, packageState) ->

            val androidPackage = packageState.androidPackage

            if (androidPackage == null || androidPackage.volumeUuid != volumeUuid) {

                return@forEach

            }

        packageNames.forEachIndexed { _, packageName ->

            val packageState = newState.externalState.packageStates[packageName]!!

            adoptPermissions(packageState, changedPermissionNames)

            addPermissionGroups(packageState)

            addPermissions(packageState, changedPermissionNames)

services/permission/java/com/android/server/permission/access/permission/PermissionService.kt

+20 −4
Original line number Diff line number Diff line
@@ -52,6 +52,7 @@ import android.util.IndentingPrintWriter 
import android.util.IntArray as GrowingIntArray

import android.util.Slog

import android.util.SparseBooleanArray

import com.android.internal.annotations.GuardedBy

import com.android.internal.compat.IPlatformCompat

import com.android.internal.logging.MetricsLogger

import com.android.internal.logging.nano.MetricsProto
@@ -123,7 +124,11 @@ class PermissionService( 
    private lateinit var onPermissionsChangeListeners: OnPermissionsChangeListeners

    private lateinit var onPermissionFlagsChangedListener: OnPermissionFlagsChangedListener



    private val storageVolumeLock = Any()

    @GuardedBy("storageVolumeLock")

    private val mountedStorageVolumes = ArraySet<String?>()

    @GuardedBy("storageVolumeLock")

    private val storageVolumePackageNames = ArrayMap<String?, MutableList<String>>()



    private lateinit var permissionControllerManager: PermissionControllerManager
@@ -2003,10 +2008,14 @@ class PermissionService( 
    }



    override fun onStorageVolumeMounted(volumeUuid: String, fingerprintChanged: Boolean) {

        service.onStorageVolumeMounted(volumeUuid, fingerprintChanged)

        synchronized(mountedStorageVolumes) {

        val packageNames: List<String>

        synchronized(storageVolumeLock) {

            // Removing the storageVolumePackageNames entry because we expect onPackageAdded()

            // to always be called before onStorageVolumeMounted().

            packageNames = storageVolumePackageNames.remove(volumeUuid) ?: emptyList()

            mountedStorageVolumes += volumeUuid

        }

        service.onStorageVolumeMounted(volumeUuid, packageNames, fingerprintChanged)

    }



    override fun onPackageAdded(
@@ -2014,7 +2023,14 @@ class PermissionService( 
        isInstantApp: Boolean,

        oldPackage: AndroidPackage?

    ) {

        synchronized(mountedStorageVolumes) {

        synchronized(storageVolumeLock) {

            // Accumulating the package names here because we want to maintain the same call order

            // of onPackageAdded() and reuse this order in onStorageVolumeAdded(). We need the

            // packages to be iterated in onStorageVolumeAdded() in the same order so that the

            // ownership of permissions is consistent.

            storageVolumePackageNames.getOrPut(packageState.volumeUuid) {

                mutableListOf()

            } += packageState.packageName

            if (packageState.volumeUuid !in mountedStorageVolumes) {

                // Wait for the storage volume to be mounted and batch the state mutation there.

                return
@@ -2046,7 +2062,7 @@ class PermissionService( 
            return

        }



        synchronized(mountedStorageVolumes) {

        synchronized(storageVolumeLock) {

            if (androidPackage.volumeUuid !in mountedStorageVolumes) {

                // Wait for the storage volume to be mounted and batch the state mutation there.

                // PackageInstalledParams won't exist when packages are being scanned instead of