diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 28d04b8689862..7e58c8b2a8225 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -356,6 +356,7 @@ import com.android.internal.util.CollectionUtils; import com.android.internal.util.ConcurrentUtils; import com.android.internal.util.DumpUtils; import com.android.internal.util.FrameworkStatsLog; +import com.android.internal.util.FunctionalUtils; import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.Preconditions; import com.android.permission.persistence.RuntimePermissionsPersistence; @@ -1686,9 +1687,8 @@ public class PackageManagerService extends IPackageManager.Stub private final DomainVerificationConnection mDomainVerificationConnection = new DomainVerificationConnection(); - private class DomainVerificationConnection implements - DomainVerificationService.Connection, DomainVerificationProxyV1.Connection, - DomainVerificationProxyV2.Connection { + private class DomainVerificationConnection implements DomainVerificationService.Connection, + DomainVerificationProxyV1.Connection, DomainVerificationProxyV2.Connection { @Override public void scheduleWriteSettings() { @@ -1732,22 +1732,77 @@ public class PackageManagerService extends IPackageManager.Stub return callingUid == getPackageUid(packageName, 0, callingUserId); } - @Nullable - @Override - public PackageSetting getPackageSettingLocked(@NonNull String pkgName) { - return PackageManagerService.this.getPackageSetting(pkgName); - } - - @Nullable - @Override - public AndroidPackage getPackageLocked(@NonNull String pkgName) { - return PackageManagerService.this.getPackage(pkgName); - } - @Nullable @Override public AndroidPackage getPackage(@NonNull String packageName) { - return getPackageLocked(packageName); + return PackageManagerService.this.getPackage(packageName); + } + + @NonNull + @Override + public void withPackageSettings(@NonNull Consumer> block) { + final Computer snapshot = snapshotComputer(); + + // This method needs to either lock or not lock consistently throughout the method, + // so if the live computer is returned, force a wrapping sync block. + if (snapshot == mLiveComputer) { + synchronized (mLock) { + block.accept(snapshot::getPackageSetting); + } + } else { + block.accept(snapshot::getPackageSetting); + } + } + + @Override + public Output withPackageSettingsReturning( + @NonNull FunctionalUtils.ThrowingFunction, Output> + block) { + final Computer snapshot = snapshotComputer(); + + // This method needs to either lock or not lock consistently throughout the method, + // so if the live computer is returned, force a wrapping sync block. + if (snapshot == mLiveComputer) { + synchronized (mLock) { + return block.apply(snapshot::getPackageSetting); + } + } else { + return block.apply(snapshot::getPackageSetting); + } + } + + @Override + public void withPackageSettingsThrowing( + @NonNull ThrowingConsumer, ExceptionType> block) + throws ExceptionType { + final Computer snapshot = snapshotComputer(); + + // This method needs to either lock or not lock consistently throughout the method, + // so if the live computer is returned, force a wrapping sync block. + if (snapshot == mLiveComputer) { + synchronized (mLock) { + block.accept(snapshot::getPackageSetting); + } + } else { + block.accept(snapshot::getPackageSetting); + } + } + + @Override + public Output + withPackageSettingsReturningThrowing(@NonNull ThrowingFunction, Output, ExceptionType> block) throws ExceptionType { + final Computer snapshot = snapshotComputer(); + + // This method needs to either lock or not lock consistently throughout the method, + // so if the live computer is returned, force a wrapping sync block. + if (snapshot == mLiveComputer) { + synchronized (mLock) { + return block.apply(snapshot::getPackageSetting); + } + } else { + return block.apply(snapshot::getPackageSetting); + } } @Override @@ -2012,7 +2067,7 @@ public class PackageManagerService extends IPackageManager.Stub // Cached attributes. The names in this class are the same as the // names in PackageManagerService; see that class for documentation. - private final Settings mSettings; + protected final Settings mSettings; private final WatchedSparseIntArray mIsolatedOwners; private final WatchedArrayMap mPackages; private final WatchedArrayMap @@ -4669,6 +4724,17 @@ public class PackageManagerService extends IPackageManager.Stub mLock = mService.mLock; } + /** + * Explicilty snapshot {@link Settings#mPackages} for cases where the caller must not lock + * in order to get package data. It is expected that the caller locks itself to be able + * to block on changes to the package data and bring itself up to date once the change + * propagates to it. Use with heavy caution. + * @return + */ + private Map snapshotPackageSettings() { + return mSettings.snapshot().mPackages; + } + public @NonNull List queryIntentServicesInternalBody(Intent intent, String resolvedType, int flags, int userId, int callingUid, String instantAppPkgName) { @@ -4800,7 +4866,7 @@ public class PackageManagerService extends IPackageManager.Stub // Compute read-only functions, based on live data. This attribute may be modified multiple // times during the PackageManagerService constructor but it should not be modified thereafter. - private Computer mLiveComputer; + private ComputerLocked mLiveComputer; // A lock-free cache for frequently called functions. private volatile Computer mSnapshotComputer; // If true, the snapshot is invalid (stale). The attribute is static since it may be diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java index 5e57ccb9cee29..0f99e1963f28d 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java @@ -28,6 +28,7 @@ import android.content.pm.PackageManager.NameNotFoundException; import android.content.pm.ResolveInfo; import android.content.pm.verify.domain.DomainVerificationInfo; import android.content.pm.verify.domain.DomainVerificationManager; +import android.content.pm.verify.domain.DomainVerificationState; import android.os.Binder; import android.os.UserHandle; import android.util.IndentingPrintWriter; @@ -35,8 +36,10 @@ import android.util.Pair; import android.util.TypedXmlPullParser; import android.util.TypedXmlSerializer; +import com.android.internal.util.FunctionalUtils; +import com.android.server.pm.PackageManagerService; import com.android.server.pm.PackageSetting; -import com.android.server.pm.parsing.pkg.AndroidPackage; +import com.android.server.pm.Settings; import com.android.server.pm.verify.domain.models.DomainVerificationPkgState; import com.android.server.pm.verify.domain.proxy.DomainVerificationProxy; @@ -46,6 +49,7 @@ import java.io.IOException; import java.util.List; import java.util.Set; import java.util.UUID; +import java.util.function.Consumer; import java.util.function.Function; public interface DomainVerificationManagerInternal { @@ -108,9 +112,10 @@ public interface DomainVerificationManagerInternal { int APPROVAL_LEVEL_INSTANT_APP = 5; /** - * Defines the possible values for {@link #approvalLevelForDomain(PackageSetting, Intent, int)} - * which sorts packages by approval priority. A higher numerical value means the package should - * override all lower values. This means that comparison using less/greater than IS valid. + * Defines the possible values for + * {@link #approvalLevelForDomain(PackageSetting, Intent, List, int, int)} which sorts packages + * by approval priority. A higher numerical value means the package should override all lower + * values. This means that comparison using less/greater than IS valid. * * Negative values are possible, although not implemented, reserved if explicit disable of a * package for a domain needs to be tracked. @@ -184,7 +189,7 @@ public interface DomainVerificationManagerInternal { /** * Migrates verification state from a previous install to a new one. It is expected that the * {@link PackageSetting#getDomainSetId()} already be set to the correct value, usually from - * {@link #generateNewId()}. This will preserve {@link DomainVerificationManager#STATE_SUCCESS} + * {@link #generateNewId()}. This will preserve {@link DomainVerificationState#STATE_SUCCESS} * domains under the assumption that the new package will pass the same server side config as * the previous package, as they have matching signatures. *

@@ -276,7 +281,7 @@ public interface DomainVerificationManagerInternal { /** * Until the legacy APIs are entirely removed, returns the legacy state from the previously - * written info stored in {@link com.android.server.pm.Settings}. + * written info stored in {@link Settings}. */ int getLegacyState(@NonNull String packageName, @UserIdInt int userId); @@ -288,15 +293,12 @@ public interface DomainVerificationManagerInternal { * @param userId the specific user to print, or null to skip printing user selection * states, supports {@link android.os.UserHandle#USER_ALL} * @param pkgSettingFunction the method by which to retrieve package data; if this is called - * from {@link com.android.server.pm.PackageManagerService}, it is - * expected to pass in the snapshot of {@link PackageSetting} objects, - * or if null is passed, the manager may decide to lock {@link - * com.android.server.pm.PackageManagerService} through {@link - * Connection#getPackageSettingLocked(String)} + * from {@link PackageManagerService}, it is + * expected to pass in the snapshot of {@link PackageSetting} objects */ void printState(@NonNull IndentingPrintWriter writer, @Nullable String packageName, @Nullable @UserIdInt Integer userId, - @Nullable Function pkgSettingFunction) + @NonNull Function pkgSettingFunction) throws NameNotFoundException; @NonNull @@ -368,17 +370,46 @@ public interface DomainVerificationManagerInternal { */ void schedule(int code, @Nullable Object object); - // TODO(b/178733426): Make DomainVerificationService PMS snapshot aware so it can avoid - // locking package state at all. This can be as simple as removing this method in favor of - // accepting a PackageSetting function in at every method call, although should probably - // be abstracted to a wrapper class. - @Nullable - PackageSetting getPackageSettingLocked(@NonNull String pkgName); + /** + * Run a function block that requires access to {@link PackageSetting} data. This will + * ensure the {@link PackageManagerService} is taken before + * {@link DomainVerificationManagerInternal}'s lock is taken to avoid deadlock. + */ + void withPackageSettings(@NonNull Consumer> block); - @Nullable - AndroidPackage getPackageLocked(@NonNull String pkgName); + /** + * Variant which returns a value to the caller. + * @see #withPackageSettings(Consumer) + */ + Output withPackageSettingsReturning( + @NonNull FunctionalUtils.ThrowingFunction, Output> + block); + + /** + * Variant which throws. + * @see #withPackageSettings(Consumer) + */ + void withPackageSettingsThrowing( + @NonNull ThrowingConsumer, ExceptionType> block) + throws ExceptionType; + + /** + * Variant which returns a value to the caller and throws. + * @see #withPackageSettings(Consumer) + */ + Output withPackageSettingsReturningThrowing( + @NonNull ThrowingFunction, Output, ExceptionType> + block) throws ExceptionType; @UserIdInt int[] getAllUserIds(); + + interface ThrowingConsumer { + void accept(Input input) throws ExceptionType; + } + + interface ThrowingFunction { + Output apply(Input input) throws ExceptionType; + } } } diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java index 7e3fb53a4da4c..81adbe197ee27 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java @@ -38,6 +38,7 @@ import android.content.pm.verify.domain.DomainVerificationManager; import android.content.pm.verify.domain.DomainVerificationState; import android.content.pm.verify.domain.DomainVerificationUserState; import android.content.pm.verify.domain.IDomainVerificationManager; +import android.os.Build; import android.os.UserHandle; import android.util.ArrayMap; import android.util.ArraySet; @@ -51,10 +52,12 @@ import android.util.TypedXmlSerializer; import com.android.internal.annotations.GuardedBy; import com.android.internal.util.CollectionUtils; +import com.android.internal.util.FunctionalUtils; import com.android.server.SystemConfig; import com.android.server.SystemService; import com.android.server.compat.PlatformCompat; import com.android.server.pm.PackageSetting; +import com.android.server.pm.Settings; import com.android.server.pm.parsing.pkg.AndroidPackage; import com.android.server.pm.verify.domain.models.DomainVerificationInternalUserState; import com.android.server.pm.verify.domain.models.DomainVerificationPkgState; @@ -73,6 +76,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.UUID; +import java.util.function.Consumer; import java.util.function.Function; public class DomainVerificationService extends SystemService @@ -100,9 +104,9 @@ public class DomainVerificationService extends SystemService * immediately attached once its available. *

* Generally this should be not accessed directly. Prefer calling {@link - * #getAndValidateAttachedLocked(UUID, Set, boolean, int, Integer)}. + * #getAndValidateAttachedLocked(UUID, Set, boolean, int, Integer, Function)}. * - * @see #getAndValidateAttachedLocked(UUID, Set, boolean, int, Integer) + * @see #getAndValidateAttachedLocked(UUID, Set, boolean, int, Integer, Function) **/ @GuardedBy("mLock") @NonNull @@ -169,7 +173,12 @@ public class DomainVerificationService extends SystemService @Override public void setConnection(@NonNull Connection connection) { - mConnection = connection; + if (Build.IS_USERDEBUG || Build.IS_ENG) { + mConnection = new LockSafeConnection(connection); + } else { + mConnection = connection; + } + mEnforcer.setCallback(mConnection); } @@ -250,41 +259,44 @@ public class DomainVerificationService extends SystemService public DomainVerificationInfo getDomainVerificationInfo(@NonNull String packageName) throws NameNotFoundException { mEnforcer.assertApprovedQuerent(mConnection.getCallingUid(), mProxy); - synchronized (mLock) { - AndroidPackage pkg = mConnection.getPackageLocked(packageName); - if (pkg == null) { - throw DomainVerificationUtils.throwPackageUnavailable(packageName); + return mConnection.withPackageSettingsReturningThrowing(pkgSettings -> { + synchronized (mLock) { + PackageSetting pkgSetting = pkgSettings.apply(packageName); + AndroidPackage pkg = pkgSetting == null ? null : pkgSetting.getPkg(); + if (pkg == null) { + throw DomainVerificationUtils.throwPackageUnavailable(packageName); + } + + DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); + if (pkgState == null) { + return null; + } + + ArrayMap hostToStateMap = new ArrayMap<>(pkgState.getStateMap()); + + // TODO(b/159952358): Should the domain list be cached? + ArraySet domains = mCollector.collectValidAutoVerifyDomains(pkg); + if (domains.isEmpty()) { + return null; + } + + int size = domains.size(); + for (int index = 0; index < size; index++) { + hostToStateMap.putIfAbsent(domains.valueAt(index), + DomainVerificationState.STATE_NO_RESPONSE); + } + + final int mapSize = hostToStateMap.size(); + for (int index = 0; index < mapSize; index++) { + int internalValue = hostToStateMap.valueAt(index); + int publicValue = DomainVerificationState.convertToInfoState(internalValue); + hostToStateMap.setValueAt(index, publicValue); + } + + // TODO(b/159952358): Do not return if no values are editable (all ignored states)? + return new DomainVerificationInfo(pkgState.getId(), packageName, hostToStateMap); } - - DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); - if (pkgState == null) { - return null; - } - - ArrayMap hostToStateMap = new ArrayMap<>(pkgState.getStateMap()); - - // TODO(b/159952358): Should the domain list be cached? - ArraySet domains = mCollector.collectValidAutoVerifyDomains(pkg); - if (domains.isEmpty()) { - return null; - } - - int size = domains.size(); - for (int index = 0; index < size; index++) { - hostToStateMap.putIfAbsent(domains.valueAt(index), - DomainVerificationState.STATE_NO_RESPONSE); - } - - final int mapSize = hostToStateMap.size(); - for (int index = 0; index < mapSize; index++) { - int internalValue = hostToStateMap.valueAt(index); - int publicValue = DomainVerificationState.convertToInfoState(internalValue); - hostToStateMap.setValueAt(index, publicValue); - } - - // TODO(b/159952358): Do not return if no values are editable (all ignored states)? - return new DomainVerificationInfo(pkgState.getId(), packageName, hostToStateMap); - } + }); } @DomainVerificationManager.Error @@ -307,44 +319,47 @@ public class DomainVerificationService extends SystemService @NonNull Set domains, int state) throws NameNotFoundException { mEnforcer.assertApprovedVerifier(callingUid, mProxy); - synchronized (mLock) { - List verifiedDomains = new ArrayList<>(); + return mConnection.withPackageSettingsReturningThrowing(pkgSettings -> { + synchronized (mLock) { + List verifiedDomains = new ArrayList<>(); - GetAttachedResult result = getAndValidateAttachedLocked(domainSetId, domains, - true /* forAutoVerify */, callingUid, null /* userId */); - if (result.isError()) { - return result.getErrorCode(); - } - - DomainVerificationPkgState pkgState = result.getPkgState(); - ArrayMap stateMap = pkgState.getStateMap(); - for (String domain : domains) { - Integer previousState = stateMap.get(domain); - if (previousState != null - && !DomainVerificationState.isModifiable(previousState)) { - continue; + GetAttachedResult result = getAndValidateAttachedLocked(domainSetId, domains, + true /* forAutoVerify */, callingUid, null /* userId */, + pkgSettings); + if (result.isError()) { + return result.getErrorCode(); } - if (DomainVerificationState.isVerified(state)) { - verifiedDomains.add(domain); + DomainVerificationPkgState pkgState = result.getPkgState(); + ArrayMap stateMap = pkgState.getStateMap(); + for (String domain : domains) { + Integer previousState = stateMap.get(domain); + if (previousState != null + && !DomainVerificationState.isModifiable(previousState)) { + continue; + } + + if (DomainVerificationState.isVerified(state)) { + verifiedDomains.add(domain); + } + + stateMap.put(domain, state); } - stateMap.put(domain, state); + int size = verifiedDomains.size(); + for (int index = 0; index < size; index++) { + removeUserStatesForDomain(verifiedDomains.get(index)); + } } - int size = verifiedDomains.size(); - for (int index = 0; index < size; index++) { - removeUserStatesForDomain(verifiedDomains.get(index)); - } - } - - mConnection.scheduleWriteSettings(); - return DomainVerificationManager.STATUS_OK; + mConnection.scheduleWriteSettings(); + return DomainVerificationManager.STATUS_OK; + }); } @Override public void setDomainVerificationStatusInternal(@Nullable String packageName, int state, - @Nullable ArraySet domains) throws NameNotFoundException { + @Nullable final ArraySet domains) throws NameNotFoundException { mEnforcer.assertInternal(mConnection.getCallingUid()); switch (state) { @@ -359,31 +374,61 @@ public class DomainVerificationService extends SystemService } ArraySet verifiedDomains = new ArraySet<>(); - if (packageName == null) { - synchronized (mLock) { - ArraySet validDomains = new ArraySet<>(); + mConnection.withPackageSettings(pkgSettings -> { + synchronized (mLock) { + ArraySet validDomains = new ArraySet<>(); - int size = mAttachedPkgStates.size(); - for (int index = 0; index < size; index++) { - DomainVerificationPkgState pkgState = mAttachedPkgStates.valueAt(index); - String pkgName = pkgState.getPackageName(); - PackageSetting pkgSetting = mConnection.getPackageSettingLocked(pkgName); + int size = mAttachedPkgStates.size(); + for (int index = 0; index < size; index++) { + DomainVerificationPkgState pkgState = mAttachedPkgStates.valueAt(index); + String pkgName = pkgState.getPackageName(); + PackageSetting pkgSetting = pkgSettings.apply(pkgName); + if (pkgSetting == null || pkgSetting.getPkg() == null) { + continue; + } + + AndroidPackage pkg = pkgSetting.getPkg(); + + validDomains.clear(); + + ArraySet autoVerifyDomains = + mCollector.collectValidAutoVerifyDomains(pkg); + if (domains == null) { + validDomains.addAll(autoVerifyDomains); + } else { + validDomains.addAll(domains); + validDomains.retainAll(autoVerifyDomains); + } + + if (DomainVerificationState.isVerified(state)) { + verifiedDomains.addAll(validDomains); + } + + setDomainVerificationStatusInternal(pkgState, state, validDomains); + } + } + }); + } else { + mConnection.withPackageSettingsThrowing(pkgSettings -> { + synchronized (mLock) { + DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); + if (pkgState == null) { + throw DomainVerificationUtils.throwPackageUnavailable(packageName); + } + + PackageSetting pkgSetting = pkgSettings.apply(packageName); if (pkgSetting == null || pkgSetting.getPkg() == null) { - continue; + throw DomainVerificationUtils.throwPackageUnavailable(packageName); } AndroidPackage pkg = pkgSetting.getPkg(); - - validDomains.clear(); - - ArraySet autoVerifyDomains = - mCollector.collectValidAutoVerifyDomains(pkg); + final ArraySet validDomains; if (domains == null) { - validDomains.addAll(autoVerifyDomains); + validDomains = mCollector.collectValidAutoVerifyDomains(pkg); } else { - validDomains.addAll(domains); - validDomains.retainAll(autoVerifyDomains); + validDomains = domains; + validDomains.retainAll(mCollector.collectValidAutoVerifyDomains(pkg)); } if (DomainVerificationState.isVerified(state)) { @@ -392,32 +437,7 @@ public class DomainVerificationService extends SystemService setDomainVerificationStatusInternal(pkgState, state, validDomains); } - } - } else { - synchronized (mLock) { - DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); - if (pkgState == null) { - throw DomainVerificationUtils.throwPackageUnavailable(packageName); - } - - PackageSetting pkgSetting = mConnection.getPackageSettingLocked(packageName); - if (pkgSetting == null || pkgSetting.getPkg() == null) { - throw DomainVerificationUtils.throwPackageUnavailable(packageName); - } - - AndroidPackage pkg = pkgSetting.getPkg(); - if (domains == null) { - domains = mCollector.collectValidAutoVerifyDomains(pkg); - } else { - domains.retainAll(mCollector.collectValidAutoVerifyDomains(pkg)); - } - - if (DomainVerificationState.isVerified(state)) { - verifiedDomains.addAll(domains); - } - - setDomainVerificationStatusInternal(pkgState, state, domains); - } + }); } // Mirror SystemApi behavior of revoking user selection for approved domains. @@ -518,44 +538,48 @@ public class DomainVerificationService extends SystemService public int setDomainVerificationUserSelection(@NonNull UUID domainSetId, @NonNull Set domains, boolean enabled, @UserIdInt int userId) throws NameNotFoundException { - synchronized (mLock) { - final int callingUid = mConnection.getCallingUid(); - // Pass null for package name here and do the app visibility enforcement inside - // getAndValidateAttachedLocked instead, since this has to fail with the same invalid - // ID reason if the target app is invisible - if (!mEnforcer.assertApprovedUserSelector(callingUid, mConnection.getCallingUserId(), - null /* packageName */, userId)) { - return DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID; - } + final int callingUid = mConnection.getCallingUid(); + // Pass null for package name here and do the app visibility enforcement inside + // getAndValidateAttachedLocked instead, since this has to fail with the same invalid + // ID reason if the target app is invisible + if (!mEnforcer.assertApprovedUserSelector(callingUid, mConnection.getCallingUserId(), + null /* packageName */, userId)) { + return DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID; + } - GetAttachedResult result = getAndValidateAttachedLocked(domainSetId, domains, - false /* forAutoVerify */, callingUid, userId); - if (result.isError()) { - return result.getErrorCode(); - } + return mConnection.withPackageSettingsReturningThrowing(pkgSettings -> { + synchronized (mLock) { + GetAttachedResult result = getAndValidateAttachedLocked(domainSetId, domains, + false /* forAutoVerify */, callingUid, userId, pkgSettings); + if (result.isError()) { + return result.getErrorCode(); + } - DomainVerificationPkgState pkgState = result.getPkgState(); - DomainVerificationInternalUserState userState = pkgState.getOrCreateUserState(userId); + DomainVerificationPkgState pkgState = result.getPkgState(); + DomainVerificationInternalUserState userState = pkgState.getOrCreateUserState( + userId); - // Disable other packages if approving this one. Note that this check is only done for - // enabling. This allows an escape hatch in case multiple packages somehow get selected. - // They can be disabled without blocking in a circular dependency. - if (enabled) { - int statusCode = revokeOtherUserSelections(userState, userId, domains); - if (statusCode != DomainVerificationManager.STATUS_OK) { - return statusCode; + // Disable other packages if approving this one. Note that this check is only done + // for enabling. This allows an escape hatch in case multiple packages somehow get + // selected. They can be disabled without blocking in a circular dependency. + if (enabled) { + int statusCode = revokeOtherUserSelectionsLocked(userState, userId, domains, + pkgSettings); + if (statusCode != DomainVerificationManager.STATUS_OK) { + return statusCode; + } + } + + if (enabled) { + userState.addHosts(domains); + } else { + userState.removeHosts(domains); } } - if (enabled) { - userState.addHosts(domains); - } else { - userState.removeHosts(domains); - } - } - - mConnection.scheduleWriteSettings(); - return DomainVerificationManager.STATUS_OK; + mConnection.scheduleWriteSettings(); + return DomainVerificationManager.STATUS_OK; + }); } @Override @@ -563,52 +587,57 @@ public class DomainVerificationService extends SystemService @NonNull String packageName, boolean enabled, @Nullable ArraySet domains) throws NameNotFoundException { mEnforcer.assertInternal(mConnection.getCallingUid()); + mConnection.withPackageSettingsThrowing(pkgSettings -> { + synchronized (mLock) { + DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); + if (pkgState == null) { + throw DomainVerificationUtils.throwPackageUnavailable(packageName); + } - synchronized (mLock) { - DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); - if (pkgState == null) { - throw DomainVerificationUtils.throwPackageUnavailable(packageName); - } + PackageSetting pkgSetting = pkgSettings.apply(packageName); + AndroidPackage pkg = pkgSetting == null ? null : pkgSetting.getPkg(); + if (pkg == null) { + throw DomainVerificationUtils.throwPackageUnavailable(packageName); + } - PackageSetting pkgSetting = mConnection.getPackageSettingLocked(packageName); - AndroidPackage pkg = pkgSetting == null ? null : pkgSetting.getPkg(); - if (pkg == null) { - throw DomainVerificationUtils.throwPackageUnavailable(packageName); - } + Set validDomains = + domains == null ? mCollector.collectAllWebDomains(pkg) : domains; - Set validDomains = - domains == null ? mCollector.collectAllWebDomains(pkg) : domains; + validDomains.retainAll(mCollector.collectAllWebDomains(pkg)); - validDomains.retainAll(mCollector.collectAllWebDomains(pkg)); - - if (userId == UserHandle.USER_ALL) { - for (int aUserId : mConnection.getAllUserIds()) { + if (userId == UserHandle.USER_ALL) { + for (int aUserId : mConnection.getAllUserIds()) { + DomainVerificationInternalUserState userState = + pkgState.getOrCreateUserState(aUserId); + revokeOtherUserSelectionsLocked(userState, aUserId, validDomains, + pkgSettings); + if (enabled) { + userState.addHosts(validDomains); + } else { + userState.removeHosts(validDomains); + } + } + } else { DomainVerificationInternalUserState userState = - pkgState.getOrCreateUserState(aUserId); - revokeOtherUserSelections(userState, aUserId, validDomains); + pkgState.getOrCreateUserState(userId); + revokeOtherUserSelectionsLocked(userState, userId, validDomains, pkgSettings); if (enabled) { userState.addHosts(validDomains); } else { userState.removeHosts(validDomains); } } - } else { - DomainVerificationInternalUserState userState = - pkgState.getOrCreateUserState(userId); - revokeOtherUserSelections(userState, userId, validDomains); - if (enabled) { - userState.addHosts(validDomains); - } else { - userState.removeHosts(validDomains); - } } - } + }); mConnection.scheduleWriteSettings(); } - private int revokeOtherUserSelections(@NonNull DomainVerificationInternalUserState userState, - @UserIdInt int userId, @NonNull Set domains) { + @GuardedBy("mLock") + private int revokeOtherUserSelectionsLocked( + @NonNull DomainVerificationInternalUserState userState, @UserIdInt int userId, + @NonNull Set domains, + @NonNull Function pkgSettingFunction) { // Cache the approved packages from the 1st pass because the search is expensive ArrayMap> domainToApprovedPackages = new ArrayMap<>(); @@ -617,8 +646,8 @@ public class DomainVerificationService extends SystemService continue; } - Pair, Integer> packagesToLevel = getApprovedPackages(domain, - userId, APPROVAL_LEVEL_NONE + 1, mConnection::getPackageSettingLocked); + Pair, Integer> packagesToLevel = getApprovedPackagesLocked(domain, + userId, APPROVAL_LEVEL_NONE + 1, pkgSettingFunction); int highestApproval = packagesToLevel.second; if (highestApproval > APPROVAL_LEVEL_SELECTION) { return DomainVerificationManager.ERROR_UNABLE_TO_APPROVE; @@ -663,46 +692,52 @@ public class DomainVerificationService extends SystemService mConnection.getCallingUserId(), packageName, userId)) { throw DomainVerificationUtils.throwPackageUnavailable(packageName); } - synchronized (mLock) { - AndroidPackage pkg = mConnection.getPackageLocked(packageName); - if (pkg == null) { - throw DomainVerificationUtils.throwPackageUnavailable(packageName); - } - DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); - if (pkgState == null) { - return null; - } - - ArraySet webDomains = mCollector.collectAllWebDomains(pkg); - int webDomainsSize = webDomains.size(); - - Map domains = new ArrayMap<>(webDomainsSize); - ArrayMap stateMap = pkgState.getStateMap(); - DomainVerificationInternalUserState userState = pkgState.getUserState(userId); - Set enabledHosts = userState == null ? emptySet() : userState.getEnabledHosts(); - - for (int index = 0; index < webDomainsSize; index++) { - String host = webDomains.valueAt(index); - Integer state = stateMap.get(host); - - int domainState; - if (state != null && DomainVerificationState.isVerified(state)) { - domainState = DomainVerificationUserState.DOMAIN_STATE_VERIFIED; - } else if (enabledHosts.contains(host)) { - domainState = DomainVerificationUserState.DOMAIN_STATE_SELECTED; - } else { - domainState = DomainVerificationUserState.DOMAIN_STATE_NONE; + return mConnection.withPackageSettingsReturningThrowing(pkgSettings -> { + synchronized (mLock) { + PackageSetting pkgSetting = pkgSettings.apply(packageName); + AndroidPackage pkg = pkgSetting == null ? null : pkgSetting.getPkg(); + if (pkg == null) { + throw DomainVerificationUtils.throwPackageUnavailable(packageName); } - domains.put(host, domainState); + DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); + if (pkgState == null) { + return null; + } + + ArraySet webDomains = mCollector.collectAllWebDomains(pkg); + int webDomainsSize = webDomains.size(); + + Map domains = new ArrayMap<>(webDomainsSize); + ArrayMap stateMap = pkgState.getStateMap(); + DomainVerificationInternalUserState userState = pkgState.getUserState(userId); + Set enabledHosts = + userState == null ? emptySet() : userState.getEnabledHosts(); + + for (int index = 0; index < webDomainsSize; index++) { + String host = webDomains.valueAt(index); + Integer state = stateMap.get(host); + + int domainState; + if (state != null && DomainVerificationState.isVerified(state)) { + domainState = DomainVerificationUserState.DOMAIN_STATE_VERIFIED; + } else if (enabledHosts.contains(host)) { + domainState = DomainVerificationUserState.DOMAIN_STATE_SELECTED; + } else { + domainState = DomainVerificationUserState.DOMAIN_STATE_NONE; + } + + domains.put(host, domainState); + } + + boolean linkHandlingAllowed = + userState == null || userState.isLinkHandlingAllowed(); + + return new DomainVerificationUserState(pkgState.getId(), packageName, + UserHandle.of(userId), linkHandlingAllowed, domains); } - - boolean linkHandlingAllowed = userState == null || userState.isLinkHandlingAllowed(); - - return new DomainVerificationUserState(pkgState.getId(), packageName, - UserHandle.of(userId), linkHandlingAllowed, domains); - } + }); } public List getOwnersForDomain(@NonNull String domain, @UserIdInt int userId) { @@ -710,67 +745,68 @@ public class DomainVerificationService extends SystemService userId); SparseArray> levelToPackages = new SparseArray<>(); + return mConnection.withPackageSettingsReturningThrowing(pkgSettings -> { + // First, collect the raw approval level values + synchronized (mLock) { + final int size = mAttachedPkgStates.size(); + for (int index = 0; index < size; index++) { + DomainVerificationPkgState pkgState = mAttachedPkgStates.valueAt(index); + String packageName = pkgState.getPackageName(); + PackageSetting pkgSetting = pkgSettings.apply(packageName); + if (pkgSetting == null) { + continue; + } - // First, collect the raw approval level values - synchronized (mLock) { - final int size = mAttachedPkgStates.size(); + int level = approvalLevelForDomain(pkgSetting, domain, userId, domain); + if (level <= APPROVAL_LEVEL_NONE) { + continue; + } + List list = levelToPackages.get(level); + if (list == null) { + list = new ArrayList<>(); + levelToPackages.put(level, list); + } + list.add(packageName); + } + } + + final int size = levelToPackages.size(); + if (size == 0) { + return emptyList(); + } + + // Then sort them ascending by first installed time, with package name as tie breaker for (int index = 0; index < size; index++) { - DomainVerificationPkgState pkgState = mAttachedPkgStates.valueAt(index); - String packageName = pkgState.getPackageName(); - PackageSetting pkgSetting = mConnection.getPackageSettingLocked(packageName); - if (pkgSetting == null) { - continue; - } + levelToPackages.valueAt(index).sort((first, second) -> { + PackageSetting firstPkgSetting = pkgSettings.apply(first); + PackageSetting secondPkgSetting = pkgSettings.apply(second); - int level = approvalLevelForDomain(pkgSetting, domain, userId, domain); - if (level <= APPROVAL_LEVEL_NONE) { - continue; - } - List list = levelToPackages.get(level); - if (list == null) { - list = new ArrayList<>(); - levelToPackages.put(level, list); - } - list.add(packageName); + long firstInstallTime = + firstPkgSetting == null ? -1L : firstPkgSetting.getFirstInstallTime(); + long secondInstallTime = + secondPkgSetting == null ? -1L : secondPkgSetting.getFirstInstallTime(); + + if (firstInstallTime != secondInstallTime) { + return (int) (firstInstallTime - secondInstallTime); + } + + return first.compareToIgnoreCase(second); + }); } - } - final int size = levelToPackages.size(); - if (size == 0) { - return emptyList(); - } - - // Then sort them ascending by first installed time, with package name as the tie breaker - for (int index = 0; index < size; index++) { - levelToPackages.valueAt(index).sort((first, second) -> { - PackageSetting firstPkgSetting = mConnection.getPackageSettingLocked(first); - PackageSetting secondPkgSetting = mConnection.getPackageSettingLocked(second); - - long firstInstallTime = - firstPkgSetting == null ? -1L : firstPkgSetting.getFirstInstallTime(); - long secondInstallTime = - secondPkgSetting == null ? -1L : secondPkgSetting.getFirstInstallTime(); - - if (firstInstallTime != secondInstallTime) { - return (int) (firstInstallTime - secondInstallTime); + List owners = new ArrayList<>(); + for (int index = 0; index < size; index++) { + int level = levelToPackages.keyAt(index); + boolean overrideable = level <= APPROVAL_LEVEL_SELECTION; + List packages = levelToPackages.valueAt(index); + int packagesSize = packages.size(); + for (int packageIndex = 0; packageIndex < packagesSize; packageIndex++) { + owners.add(new DomainOwner(packages.get(packageIndex), overrideable)); } - - return first.compareToIgnoreCase(second); - }); - } - - List owners = new ArrayList<>(); - for (int index = 0; index < size; index++) { - int level = levelToPackages.keyAt(index); - boolean overrideable = level <= APPROVAL_LEVEL_SELECTION; - List packages = levelToPackages.valueAt(index); - int packagesSize = packages.size(); - for (int packageIndex = 0; packageIndex < packagesSize; packageIndex++) { - owners.add(new DomainOwner(packages.get(packageIndex), overrideable)); } - } - return owners; + return owners; + }); } @NonNull @@ -1085,21 +1121,15 @@ public class DomainVerificationService extends SystemService @Override public void printState(@NonNull IndentingPrintWriter writer, @Nullable String packageName, @Nullable Integer userId) throws NameNotFoundException { - // This method is only used by DomainVerificationShell, which doesn't lock PMS, so it's - // safe to pass mConnection directly here and lock PMS. This method is not exposed - // to the general system server/PMS. - printState(writer, packageName, userId, mConnection::getPackageSettingLocked); + mConnection.withPackageSettingsThrowing( + pkgSettings -> printState(writer, packageName, userId, pkgSettings)); } @Override public void printState(@NonNull IndentingPrintWriter writer, @Nullable String packageName, @Nullable @UserIdInt Integer userId, - @Nullable Function pkgSettingFunction) + @NonNull Function pkgSettingFunction) throws NameNotFoundException { - if (pkgSettingFunction == null) { - pkgSettingFunction = mConnection::getPackageSettingLocked; - } - synchronized (mLock) { mDebug.printState(writer, packageName, userId, pkgSettingFunction, mAttachedPkgStates); } @@ -1148,7 +1178,9 @@ public class DomainVerificationService extends SystemService @GuardedBy("mLock") private GetAttachedResult getAndValidateAttachedLocked(@NonNull UUID domainSetId, @NonNull Set domains, boolean forAutoVerify, int callingUid, - @Nullable Integer userIdForFilter) throws NameNotFoundException { + @Nullable Integer userIdForFilter, + @NonNull Function pkgSettingFunction) + throws NameNotFoundException { if (domainSetId == null) { throw new IllegalArgumentException("domainSetId cannot be null"); } @@ -1165,7 +1197,7 @@ public class DomainVerificationService extends SystemService return GetAttachedResult.error(DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID); } - PackageSetting pkgSetting = mConnection.getPackageSettingLocked(pkgName); + PackageSetting pkgSetting = pkgSettingFunction.apply(pkgName); if (pkgSetting == null || pkgSetting.getPkg() == null) { throw DomainVerificationUtils.throwPackageUnavailable(pkgName); } @@ -1253,31 +1285,33 @@ public class DomainVerificationService extends SystemService @Override public void clearDomainVerificationState(@Nullable List packageNames) { mEnforcer.assertInternal(mConnection.getCallingUid()); - synchronized (mLock) { - if (packageNames == null) { - int size = mAttachedPkgStates.size(); - for (int index = 0; index < size; index++) { - DomainVerificationPkgState pkgState = mAttachedPkgStates.valueAt(index); - String pkgName = pkgState.getPackageName(); - PackageSetting pkgSetting = mConnection.getPackageSettingLocked(pkgName); - if (pkgSetting == null || pkgSetting.getPkg() == null) { - continue; + mConnection.withPackageSettings(pkgSettings -> { + synchronized (mLock) { + if (packageNames == null) { + int size = mAttachedPkgStates.size(); + for (int index = 0; index < size; index++) { + DomainVerificationPkgState pkgState = mAttachedPkgStates.valueAt(index); + String pkgName = pkgState.getPackageName(); + PackageSetting pkgSetting = pkgSettings.apply(pkgName); + if (pkgSetting == null || pkgSetting.getPkg() == null) { + continue; + } + resetDomainState(pkgState, pkgSetting.getPkg()); } - resetDomainState(pkgState, pkgSetting.getPkg()); - } - } else { - int size = packageNames.size(); - for (int index = 0; index < size; index++) { - String pkgName = packageNames.get(index); - DomainVerificationPkgState pkgState = mAttachedPkgStates.get(pkgName); - PackageSetting pkgSetting = mConnection.getPackageSettingLocked(pkgName); - if (pkgSetting == null || pkgSetting.getPkg() == null) { - continue; + } else { + int size = packageNames.size(); + for (int index = 0; index < size; index++) { + String pkgName = packageNames.get(index); + DomainVerificationPkgState pkgState = mAttachedPkgStates.get(pkgName); + PackageSetting pkgSetting = pkgSettings.apply(pkgName); + if (pkgSetting == null || pkgSetting.getPkg() == null) { + continue; + } + resetDomainState(pkgState, pkgSetting.getPkg()); } - resetDomainState(pkgState, pkgSetting.getPkg()); } } - } + }); mConnection.scheduleWriteSettings(); } @@ -1692,8 +1726,9 @@ public class DomainVerificationService extends SystemService /** * @return the filtered list paired with the corresponding approval level */ + @GuardedBy("mLock") @NonNull - private Pair, Integer> getApprovedPackages(@NonNull String domain, + private Pair, Integer> getApprovedPackagesLocked(@NonNull String domain, @UserIdInt int userId, int minimumApproval, @NonNull Function pkgSettingFunction) { int highestApproval = minimumApproval; @@ -1792,4 +1827,93 @@ public class DomainVerificationService extends SystemService return mErrorCode; } } + + /** + * Wraps a {@link Connection} to verify that the {@link PackageSetting} calls do not hold + * {@link #mLock}, as that can cause deadlock when {@link Settings} tries to serialize state to + * disk. Only enabled if {@link Build#IS_USERDEBUG} or {@link Build#IS_ENG} is true. + */ + private class LockSafeConnection implements Connection { + + @NonNull + private final Connection mConnection; + + private LockSafeConnection(@NonNull Connection connection) { + mConnection = connection; + } + + private void enforceLocking() { + if (Thread.holdsLock(mLock)) { + Slog.wtf(TAG, "Method should not hold DVS lock when accessing package data"); + } + } + + @Override + public void withPackageSettings( + @NonNull Consumer> block) { + enforceLocking(); + mConnection.withPackageSettings(block); + } + + @Override + public Output withPackageSettingsReturning( + @NonNull FunctionalUtils.ThrowingFunction, Output> + block) { + enforceLocking(); + return mConnection.withPackageSettingsReturning(block); + } + + @Override + public void withPackageSettingsThrowing( + @NonNull ThrowingConsumer, ExceptionType> block) + throws ExceptionType { + enforceLocking(); + mConnection.withPackageSettingsThrowing(block); + } + + @Override + public Output + withPackageSettingsReturningThrowing(@NonNull ThrowingFunction, Output, ExceptionType> block) throws ExceptionType { + enforceLocking(); + return mConnection.withPackageSettingsReturningThrowing(block); + } + + @Override + public void scheduleWriteSettings() { + mConnection.scheduleWriteSettings(); + } + + @Override + public int getCallingUid() { + return mConnection.getCallingUid(); + } + + @Override + @UserIdInt + public int getCallingUserId() { + return mConnection.getCallingUserId(); + } + + @Override + public void schedule(int code, @Nullable Object object) { + mConnection.schedule(code, object); + } + + @Override + @UserIdInt + public int[] getAllUserIds() { + return mConnection.getAllUserIds(); + } + + @Override + public boolean filterAppAccess(@NonNull String packageName, int callingUid, int userId) { + return mConnection.filterAppAccess(packageName, callingUid, userId); + } + + @Override + public boolean doesUserExist(int userId) { + return mConnection.doesUserExist(userId); + } + } } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt index 01a76eea2b0d8..2d852e5017b45 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt @@ -31,6 +31,7 @@ import android.util.SparseArray import androidx.test.platform.app.InstrumentationRegistry import com.android.server.pm.PackageSetting import com.android.server.pm.parsing.pkg.AndroidPackage +import com.android.server.pm.test.verify.domain.DomainVerificationTestUtils.mockPackageSettings import com.android.server.pm.verify.domain.DomainVerificationEnforcer import com.android.server.pm.verify.domain.DomainVerificationManagerInternal import com.android.server.pm.verify.domain.DomainVerificationService @@ -98,10 +99,13 @@ class DomainVerificationEnforcerTest { mockThrowOnUnmocked { whenever(callingUid) { callingUidInt.get() } whenever(callingUserId) { callingUserIdInt.get() } - whenever(getPackageSettingLocked(VISIBLE_PKG)) { visiblePkgSetting } - whenever(getPackageLocked(VISIBLE_PKG)) { visiblePkg } - whenever(getPackageSettingLocked(INVISIBLE_PKG)) { invisiblePkgSetting } - whenever(getPackageLocked(INVISIBLE_PKG)) { invisiblePkg } + mockPackageSettings { + when (it) { + VISIBLE_PKG -> visiblePkgSetting + INVISIBLE_PKG -> invisiblePkgSetting + else -> null + } + } whenever(schedule(anyInt(), any())) whenever(scheduleWriteSettings()) whenever(filterAppAccess(eq(VISIBLE_PKG), anyInt(), anyInt())) { false } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt index f8e0c60c25df6..adc1dab1572e2 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt @@ -32,6 +32,7 @@ import android.os.Process import android.util.ArraySet import com.android.server.pm.PackageSetting import com.android.server.pm.parsing.pkg.AndroidPackage +import com.android.server.pm.test.verify.domain.DomainVerificationTestUtils.mockPackageSettings import com.android.server.pm.verify.domain.DomainVerificationService import com.android.server.testutils.mockThrowOnUnmocked import com.android.server.testutils.whenever @@ -445,11 +446,8 @@ class DomainVerificationManagerApiTest { whenever(callingUid) { Process.ROOT_UID } whenever(callingUserId) { 0 } - whenever(getPackageSettingLocked(anyString())) { - pkgSettingFunction(arguments[0] as String) - } - whenever(getPackageLocked(anyString())) { - pkgSettingFunction(arguments[0] as String)?.getPkg() + mockPackageSettings { + pkgSettingFunction(it) } }) } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt index 5f601e2648cb5..8a549731af3a2 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt @@ -36,6 +36,7 @@ import android.util.ArraySet import android.util.Xml import com.android.server.pm.PackageSetting import com.android.server.pm.parsing.pkg.AndroidPackage +import com.android.server.pm.test.verify.domain.DomainVerificationTestUtils.mockPackageSettings import com.android.server.pm.verify.domain.DomainVerificationService import com.android.server.testutils.mockThrowOnUnmocked import com.android.server.testutils.whenever @@ -380,11 +381,8 @@ class DomainVerificationPackageTest { whenever(callingUid) { Process.ROOT_UID } whenever(callingUserId) { 0 } - whenever(getPackageSettingLocked(anyString())) { - pkgSettingFunction(arguments[0] as String)!! - } - whenever(getPackageLocked(anyString())) { - pkgSettingFunction(arguments[0] as String)!!.getPkg() + mockPackageSettings { + pkgSettingFunction(it) } }) } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt index 1cf57c41145a0..26e8d286cabe9 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt @@ -29,6 +29,7 @@ import android.util.ArraySet import android.util.SparseArray import com.android.server.pm.PackageSetting import com.android.server.pm.parsing.pkg.AndroidPackage +import com.android.server.pm.test.verify.domain.DomainVerificationTestUtils.mockPackageSettings import com.android.server.pm.verify.domain.DomainVerificationManagerInternal import com.android.server.pm.verify.domain.DomainVerificationService import com.android.server.pm.verify.domain.proxy.DomainVerificationProxy @@ -256,8 +257,12 @@ class DomainVerificationSettingsMutationTest { mockThrowOnUnmocked { whenever(callingUid) { TEST_UID } whenever(callingUserId) { TEST_USER_ID } - whenever(getPackageSettingLocked(TEST_PKG)) { mockPkgSetting() } - whenever(getPackageLocked(TEST_PKG)) { mockPkg() } + mockPackageSettings { + when (it) { + TEST_PKG -> mockPkgSetting() + else -> null + } + } whenever(schedule(anyInt(), any())) whenever(scheduleWriteSettings()) diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationTestUtils.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationTestUtils.kt new file mode 100644 index 0000000000000..e1da727fb64ff --- /dev/null +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationTestUtils.kt @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2021 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.pm.test.verify.domain + +import com.android.internal.util.FunctionalUtils +import com.android.server.pm.PackageSetting +import com.android.server.pm.verify.domain.DomainVerificationManagerInternal +import com.android.server.testutils.whenever +import org.mockito.ArgumentMatchers.any +import java.util.function.Consumer +import java.util.function.Function + +internal object DomainVerificationTestUtils { + + @Suppress("UNCHECKED_CAST") + fun DomainVerificationManagerInternal.Connection.mockPackageSettings( + block: (String) -> PackageSetting? + ) { + whenever(withPackageSettings(any())) { + (arguments[0] as Consumer>).accept { block(it) } + } + whenever(withPackageSettingsReturning(any())) { + (arguments[0] as FunctionalUtils.ThrowingFunction, *>) + .apply { block(it) } + } + whenever(withPackageSettingsThrowing(any())) { + (arguments[0] as DomainVerificationManagerInternal.Connection.ThrowingConsumer< + Function, *>) + .accept { block(it) } + } + whenever(withPackageSettingsReturningThrowing(any())) { + (arguments[0] as DomainVerificationManagerInternal.Connection.ThrowingFunction< + Function, *, *>) + .apply { block(it) } + } + } +} diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt index 474e8012d43bd..52ae7a5356571 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt @@ -30,6 +30,7 @@ import android.os.Process import android.util.ArraySet import com.android.server.pm.PackageSetting import com.android.server.pm.parsing.pkg.AndroidPackage +import com.android.server.pm.test.verify.domain.DomainVerificationTestUtils.mockPackageSettings import com.android.server.pm.verify.domain.DomainVerificationService import com.android.server.testutils.mockThrowOnUnmocked import com.android.server.testutils.whenever @@ -83,10 +84,13 @@ class DomainVerificationUserStateOverrideTest { // Need to provide an internal UID so some permission checks are ignored whenever(callingUid) { Process.ROOT_UID } whenever(callingUserId) { 0 } - whenever(getPackageSettingLocked(PKG_ONE)) { pkg1 } - whenever(getPackageSettingLocked(PKG_TWO)) { pkg2 } - whenever(getPackageLocked(PKG_ONE)) { pkg1.getPkg() } - whenever(getPackageLocked(PKG_TWO)) { pkg2.getPkg() } + mockPackageSettings { + when (it) { + PKG_ONE -> pkg1 + PKG_TWO -> pkg2 + else -> null + } + } }) addPackage(pkg1) addPackage(pkg2)