Avoid locking PMS when printing from DomainVerificationService

Passes in a String -> PackageSetting function rather than calling into
PackageManagerService directly and potentially taking the locked
computer.

Note that this change itself does not prevent locking, just allows that
change to be made.

Bug: 179461758

Test: builds, mostly debug functionality

Change-Id: Ib9172e797edd26c5cd8621c16e2e06181e9b91a4

services/core/java/com/android/server/pm/PackageManagerService.java

@@ -24,7 +24,6 @@ import static android.Manifest.permission.SET_HARMFUL_APP_WARNINGS;
 import static android.app.AppOpsManager.MODE_DEFAULT;
 import static android.app.AppOpsManager.MODE_IGNORED;
 import static android.content.Intent.ACTION_MAIN;
-import static android.content.Intent.CATEGORY_BROWSABLE;
 import static android.content.Intent.CATEGORY_DEFAULT;
 import static android.content.Intent.CATEGORY_HOME;
 import static android.content.Intent.EXTRA_LONG_VERSION_CODE;
@@ -68,11 +67,7 @@ import static android.content.pm.PackageManager.INSTALL_REASON_DEVICE_RESTORE;
 import static android.content.pm.PackageManager.INSTALL_REASON_DEVICE_SETUP;
 import static android.content.pm.PackageManager.INSTALL_STAGED;
 import static android.content.pm.PackageManager.INSTALL_SUCCEEDED;
-import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ALWAYS;
-import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ALWAYS_ASK;
-import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ASK;
 import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_NEVER;
-import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_UNDEFINED;
 import static android.content.pm.PackageManager.MATCH_ALL;
 import static android.content.pm.PackageManager.MATCH_ANY_USER;
 import static android.content.pm.PackageManager.MATCH_APEX;
@@ -377,11 +372,6 @@ import com.android.server.pm.dex.DexManager;
 import com.android.server.pm.dex.DexoptOptions;
 import com.android.server.pm.dex.PackageDexUsage;
 import com.android.server.pm.dex.ViewCompiler;
-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;
-import com.android.server.pm.verify.domain.proxy.DomainVerificationProxyV1;
-import com.android.server.pm.verify.domain.proxy.DomainVerificationProxyV2;
 import com.android.server.pm.parsing.PackageCacher;
 import com.android.server.pm.parsing.PackageInfoUtils;
 import com.android.server.pm.parsing.PackageParser2;
@@ -395,6 +385,11 @@ import com.android.server.pm.permission.LegacyPermissionManagerService;
 import com.android.server.pm.permission.Permission;
 import com.android.server.pm.permission.PermissionManagerService;
 import com.android.server.pm.permission.PermissionManagerServiceInternal;
+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;
+import com.android.server.pm.verify.domain.proxy.DomainVerificationProxyV1;
+import com.android.server.pm.verify.domain.proxy.DomainVerificationProxyV2;
 import com.android.server.rollback.RollbackManagerInternal;
 import com.android.server.security.VerityUtils;
 import com.android.server.storage.DeviceStorageMonitorInternal;
@@ -404,7 +399,6 @@ import com.android.server.utils.Watchable;
 import com.android.server.utils.Watched;
 import com.android.server.utils.WatchedArrayMap;
 import com.android.server.utils.WatchedSparseBooleanArray;
-import com.android.server.utils.WatchedSparseIntArray;
 import com.android.server.utils.Watcher;
 import com.android.server.wm.ActivityTaskManagerInternal;
 
@@ -466,7 +460,6 @@ import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
-import java.util.function.Supplier;
 
 /**
  * Keep track of all those APKs everywhere.
@@ -23937,7 +23930,8 @@ public class PackageManagerService extends IPackageManager.Stub
                 writer.println("Domain verification status:");
                 writer.increaseIndent();
                 try {
-                    mDomainVerificationManager.printState(writer, packageName, UserHandle.USER_ALL);
+                    mDomainVerificationManager.printState(writer, packageName, UserHandle.USER_ALL,
+                            mSettings::getPackageLPr);
                 } catch (PackageManager.NameNotFoundException e) {
                     pw.println("Failure printing domain verification information");
                     Slog.e(TAG, "Failure printing domain verification information", e);

services/core/java/com/android/server/pm/verify/domain/DomainVerificationDebug.java

@@ -38,6 +38,9 @@ import com.android.server.pm.verify.domain.models.DomainVerificationStateMap;
 import com.android.server.pm.verify.domain.models.DomainVerificationUserState;
 
 import java.util.Arrays;
+import java.util.Objects;
+import java.util.Set;
+import java.util.function.Function;
 
 @SuppressWarnings("PointlessBooleanExpression")
 public class DomainVerificationDebug {
@@ -62,7 +65,7 @@ public class DomainVerificationDebug {
 
     public void printState(@NonNull IndentingPrintWriter writer, @Nullable String packageName,
             @Nullable @UserIdInt Integer userId,
-            @NonNull DomainVerificationService.Connection connection,
+            @NonNull Function<String, PackageSetting> pkgSettingFunction,
             @NonNull DomainVerificationStateMap<DomainVerificationPkgState> stateMap)
             throws NameNotFoundException {
         ArrayMap<String, Integer> reusedMap = new ArrayMap<>();
@@ -73,7 +76,7 @@ public class DomainVerificationDebug {
             for (int index = 0; index < size; index++) {
                 DomainVerificationPkgState pkgState = stateMap.valueAt(index);
                 String pkgName = pkgState.getPackageName();
-                PackageSetting pkgSetting = connection.getPackageSettingLocked(pkgName);
+                PackageSetting pkgSetting = pkgSettingFunction.apply(pkgName);
                 if (pkgSetting == null || pkgSetting.getPkg() == null) {
                     continue;
                 }
@@ -89,7 +92,7 @@ public class DomainVerificationDebug {
                 throw DomainVerificationUtils.throwPackageUnavailable(packageName);
             }
 
-            PackageSetting pkgSetting = connection.getPackageSettingLocked(packageName);
+            PackageSetting pkgSetting = pkgSettingFunction.apply(packageName);
             if (pkgSetting == null || pkgSetting.getPkg() == null) {
                 throw DomainVerificationUtils.throwPackageUnavailable(packageName);
             }

services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java

@@ -32,15 +32,16 @@ import android.util.TypedXmlPullParser;
 import android.util.TypedXmlSerializer;
 
 import com.android.server.pm.PackageSetting;
+import com.android.server.pm.parsing.pkg.AndroidPackage;
 import com.android.server.pm.verify.domain.models.DomainVerificationPkgState;
 import com.android.server.pm.verify.domain.proxy.DomainVerificationProxy;
-import com.android.server.pm.parsing.pkg.AndroidPackage;
 
 import org.xmlpull.v1.XmlPullParserException;
 
 import java.io.IOException;
 import java.util.Set;
 import java.util.UUID;
+import java.util.function.Function;
 
 public interface DomainVerificationManagerInternal extends DomainVerificationManager {
 
@@ -191,12 +192,21 @@ public interface DomainVerificationManagerInternal extends DomainVerificationMan
     /**
      * Print the verification state and user selection state of a package.
      *
-     * @param packageName the package whose state to change, or all packages if none is specified
-     * @param userId      the specific user to print, or null to skip printing user selection
-     *                    states, supports {@link android.os.UserHandle#USER_ALL}
+     * @param packageName        the package whose state to change, or all packages if none is
+     *                           specified
+     * @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)}
      */
     void printState(@NonNull IndentingPrintWriter writer, @Nullable String packageName,
-            @Nullable @UserIdInt Integer userId) throws NameNotFoundException;
+            @Nullable @UserIdInt Integer userId,
+            @Nullable Function<String, PackageSetting> pkgSettingFunction)
+            throws NameNotFoundException;
 
     @NonNull
     DomainVerificationShell getShell();
@@ -225,7 +235,7 @@ public interface DomainVerificationManagerInternal extends DomainVerificationMan
             throws IllegalArgumentException, NameNotFoundException;
 
 
-    interface Connection {
+    interface Connection extends Function<String, PackageSetting> {
 
         /**
          * Notify that a settings change has been made and that eventually
@@ -249,10 +259,19 @@ public interface DomainVerificationManagerInternal extends DomainVerificationMan
          */
         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);
 
         @Nullable
         AndroidPackage getPackageLocked(@NonNull String pkgName);
+
+        @Override
+        default PackageSetting apply(@NonNull String pkgName) {
+            return getPackageSettingLocked(pkgName);
+        }
     }
 }

services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java

@@ -64,6 +64,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
+import java.util.function.Function;
 
 public class DomainVerificationService extends SystemService
         implements DomainVerificationManagerInternal, DomainVerificationShell.Callback {
@@ -890,9 +891,24 @@ public class DomainVerificationService extends SystemService
 
     @Override
     public void printState(@NonNull IndentingPrintWriter writer, @Nullable String packageName,
-            @Nullable @UserIdInt Integer userId) throws NameNotFoundException {
+            @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);
+    }
+
+    @Override
+    public void printState(@NonNull IndentingPrintWriter writer, @Nullable String packageName,
+            @Nullable @UserIdInt Integer userId,
+            @Nullable Function<String, PackageSetting> pkgSettingFunction)
+            throws NameNotFoundException {
+        if (pkgSettingFunction == null) {
+            pkgSettingFunction = mConnection;
+        }
+
         synchronized (mLock) {
-            mDebug.printState(writer, packageName, userId, mConnection, mAttachedPkgStates);
+            mDebug.printState(writer, packageName, userId, pkgSettingFunction, mAttachedPkgStates);
         }
     }