From 5c2bfbaea673c8329e3243910401f43f682bbf30 Mon Sep 17 00:00:00 2001
From: JW Wang <wangchun@google.com>
Date: Thu, 13 Aug 2020 15:26:59 +0800
Subject: [PATCH] Reject inconsistent rollback/staged settings ASAP (1/n)

Inspired by
https://googleplex-android-review.git.corp.google.com/c/platform/frameworks/base/+/12343143/1/services/core/java/com/android/server/pm/PackageInstallerSession.java#1555

Since the rollback/staged settings won't change after a session
is created, we can reject such invalid cases in addChildSessionId()
without waiting for it to fail until commit.

This also simplify the error handling in later stages how to fail
and clean up properly when some of the children fail.

Bug: 163976510
Test: N/A, will be added in next CL
Change-Id: I1f41355fa6fce5dd1487378c4b8b396c52915e61
---
 .../server/pm/PackageInstallerSession.java    | 64 +++----------------
 1 file changed, 10 insertions(+), 54 deletions(-)

diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index c37b6e7354ab7..bc090f0aadda7 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -1488,53 +1488,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         return childSessions;
     }
 
-    /**
-     * Assert multipackage install has consistent sessions.
-     *
-     * @throws PackageManagerException if child sessions don't match parent session
-     *                                  in respect to staged and enable rollback parameters.
-     */
-    @GuardedBy("mLock")
-    private void assertMultiPackageConsistencyLocked(
-            @NonNull List<PackageInstallerSession> childSessions) throws PackageManagerException {
-        for (PackageInstallerSession childSession : childSessions) {
-            // It might be that the parent session is loaded before all of it's child sessions are,
-            // e.g. when reading sessions from XML. Those sessions will be null here, and their
-            // conformance with the multipackage params will be checked when they're loaded.
-            if (childSession == null) {
-                continue;
-            }
-            assertConsistencyWithLocked(childSession);
-        }
-    }
-
-    /**
-     * Assert consistency with the given session.
-     *
-     * @throws PackageManagerException if other sessions doesn't match this session
-     *                                  in respect to staged and enable rollback parameters.
-     */
-    @GuardedBy("mLock")
-    private void assertConsistencyWithLocked(PackageInstallerSession other)
-            throws PackageManagerException {
-        // Session groups must be consistent wrt to isStaged parameter. Non-staging session
-        // cannot be grouped with staging sessions.
-        if (this.params.isStaged != other.params.isStaged) {
-            throw new PackageManagerException(
-                PackageManager.INSTALL_FAILED_MULTIPACKAGE_INCONSISTENCY,
-                "Multipackage Inconsistency: session " + other.sessionId
-                    + " and session " + sessionId
-                    + " have inconsistent staged settings");
-        }
-        if (this.params.getEnableRollback() != other.params.getEnableRollback()) {
-            throw new PackageManagerException(
-                PackageManager.INSTALL_FAILED_MULTIPACKAGE_INCONSISTENCY,
-                "Multipackage Inconsistency: session " + other.sessionId
-                    + " and session " + sessionId
-                    + " have inconsistent rollback settings");
-        }
-    }
-
     /**
      * Seal the session to prevent further modification.
      *
@@ -1549,14 +1502,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         try {
             assertNoWriteFileTransfersOpenLocked();
             assertPreparedAndNotDestroyedLocked("sealing of session");
-
             mSealed = true;
-            List<PackageInstallerSession> childSessions = getChildSessionsLocked();
-            if (childSessions != null) {
-                assertMultiPackageConsistencyLocked(childSessions);
-            }
-        } catch (PackageManagerException e) {
-            throw onSessionValidationFailure(e);
         } catch (Throwable e) {
             // Convert all exceptions into package manager exceptions as only those are handled
             // in the code above.
@@ -3190,6 +3136,16 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
             throw new IllegalStateException("Multi-session " + childSessionId
                     + " can't be a child.");
         }
+        if (params.isStaged != childSession.params.isStaged) {
+            throw new IllegalStateException("Multipackage Inconsistency: session "
+                    + childSession.sessionId + " and session " + sessionId
+                    + " have inconsistent staged settings");
+        }
+        if (params.getEnableRollback() != childSession.params.getEnableRollback()) {
+            throw new IllegalStateException("Multipackage Inconsistency: session "
+                    + childSession.sessionId + " and session " + sessionId
+                    + " have inconsistent rollback settings");
+        }
 
         try {
             acquireTransactionLock();