From bd9f18f7ae9adf6b4773361be17f2ed836a04ce0 Mon Sep 17 00:00:00 2001 From: Winson Date: Wed, 23 Feb 2022 09:56:15 -0800 Subject: [PATCH] Take package snapshot before locking DomainVerificationService Because snapshot() can take PMS#mLock when the snapshot is invalid, there's a potential deadlock if Settings is trying to serialize into DVS. Instead, take the snapshot before locking DVS, which introduces a potential race condition, as DVS relies on its internal lock to serialize package changes, but there's not much better that can be done until mutate-time snapshots are enabled. Bug: 220994615 Test: atest com.android.server.pm.test.verify.domain Test: atest CtsDomainVerificationHostTestCases Test: atest CtsDomainVerificationDeviceMultiUserTestCases Test: atest CtsDomainVerificationDeviceStandaloneTestCases Change-Id: Ib4f4605d6b19ef14e47dffc3df3bee62a745a817 --- .../domain/DomainVerificationService.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) 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 13218eaf2ded4..67aed4506b274 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 @@ -256,8 +256,8 @@ public class DomainVerificationService extends SystemService public DomainVerificationInfo getDomainVerificationInfo(@NonNull String packageName) throws NameNotFoundException { mEnforcer.assertApprovedQuerent(mConnection.getCallingUid(), mProxy); + final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { - final Computer snapshot = mConnection.snapshot(); PackageStateInternal pkgSetting = snapshot.getPackageStateInternal(packageName); AndroidPackage pkg = pkgSetting == null ? null : pkgSetting.getPkg(); if (pkg == null) { @@ -315,8 +315,8 @@ public class DomainVerificationService extends SystemService @NonNull Set domains, int state) throws NameNotFoundException { mEnforcer.assertApprovedVerifier(callingUid, mProxy); + final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { - final Computer snapshot = mConnection.snapshot(); List verifiedDomains = new ArrayList<>(); GetAttachedResult result = getAndValidateAttachedLocked(domainSetId, domains, @@ -369,8 +369,8 @@ public class DomainVerificationService extends SystemService ArraySet verifiedDomains = new ArraySet<>(); if (packageName == null) { + final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { - final Computer snapshot = mConnection.snapshot(); ArraySet validDomains = new ArraySet<>(); int size = mAttachedPkgStates.size(); @@ -403,8 +403,8 @@ public class DomainVerificationService extends SystemService } } } else { + final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { - final Computer snapshot = mConnection.snapshot(); DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); if (pkgState == null) { throw DomainVerificationUtils.throwPackageUnavailable(packageName); @@ -539,8 +539,8 @@ public class DomainVerificationService extends SystemService return DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID; } + final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { - final Computer snapshot = mConnection.snapshot(); GetAttachedResult result = getAndValidateAttachedLocked(domainSetId, domains, false /* forAutoVerify */, callingUid, userId, snapshot); if (result.isError()) { @@ -578,8 +578,8 @@ public class DomainVerificationService extends SystemService @NonNull String packageName, boolean enabled, @Nullable ArraySet domains) throws NameNotFoundException { mEnforcer.assertInternal(mConnection.getCallingUid()); + final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { - final Computer snapshot = mConnection.snapshot(); DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); if (pkgState == null) { throw DomainVerificationUtils.throwPackageUnavailable(packageName); @@ -682,8 +682,8 @@ public class DomainVerificationService extends SystemService throw DomainVerificationUtils.throwPackageUnavailable(packageName); } + final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { - final Computer snapshot = mConnection.snapshot(); PackageStateInternal pkgSetting = snapshot.getPackageStateInternal(packageName); AndroidPackage pkg = pkgSetting == null ? null : pkgSetting.getPkg(); if (pkg == null) { @@ -1179,8 +1179,8 @@ public class DomainVerificationService extends SystemService public void printOwnersForPackage(@NonNull IndentingPrintWriter writer, @Nullable String packageName, @Nullable @UserIdInt Integer userId) throws NameNotFoundException { + final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { - final Computer snapshot = mConnection.snapshot(); if (packageName == null) { int size = mAttachedPkgStates.size(); for (int index = 0; index < size; index++) { @@ -1227,8 +1227,8 @@ public class DomainVerificationService extends SystemService @Override public void printOwnersForDomains(@NonNull IndentingPrintWriter writer, @NonNull List domains, @Nullable @UserIdInt Integer userId) { + final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { - final Computer snapshot = mConnection.snapshot(); int size = domains.size(); for (int index = 0; index < size; index++) { printOwnersForDomain(writer, domains.get(index), userId, snapshot); @@ -1403,8 +1403,8 @@ public class DomainVerificationService extends SystemService @Override public void clearDomainVerificationState(@Nullable List packageNames) { mEnforcer.assertInternal(mConnection.getCallingUid()); + final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { - final Computer snapshot = mConnection.snapshot(); if (packageNames == null) { int size = mAttachedPkgStates.size(); for (int index = 0; index < size; index++) {