Merge "Sessions committed later shouldn't cause earlier ones to fail"

5 files changed, 168 insertions, 9 deletions

diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java
index 9f8f6e4408f8..9e48ddd63c01 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerService.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerService.java
@@ -747,8 +747,8 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
                 installerAttributionTag);
         session = new PackageInstallerSession(mInternalCallback, mContext, mPm, this,
                 mInstallThread.getLooper(), mStagingManager, sessionId, userId, callingUid,
-                installSource, params, createdMillis, stageDir, stageCid, null, null, false, false,
-                false, false, null, SessionInfo.INVALID_ID, false, false, false,
+                installSource, params, createdMillis, 0L, stageDir, stageCid, null, null, false,
+                false, false, false, null, SessionInfo.INVALID_ID, false, false, false,
                 SessionInfo.STAGED_SESSION_NO_ERROR, "");
 
         synchronized (mSessions) {
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index 4ab12823dfb7..b8daa832cd2d 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -35,6 +35,7 @@ import static android.system.OsConstants.O_CREAT;
 import static android.system.OsConstants.O_RDONLY;
 import static android.system.OsConstants.O_WRONLY;
 
+import static com.android.internal.annotations.VisibleForTesting.Visibility.PACKAGE;
 import static com.android.internal.util.XmlUtils.readBitmapAttribute;
 import static com.android.internal.util.XmlUtils.readByteArrayAttribute;
 import static com.android.internal.util.XmlUtils.readStringAttribute;
@@ -132,6 +133,7 @@ import android.util.apk.ApkSignatureVerifier;
 
 import com.android.internal.R;
 import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.content.NativeLibraryHelper;
 import com.android.internal.content.PackageHelper;
 import com.android.internal.messages.nano.SystemMessageProto;
@@ -201,6 +203,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
             "installOriginatingPackageName";
     private static final String ATTR_CREATED_MILLIS = "createdMillis";
     private static final String ATTR_UPDATED_MILLIS = "updatedMillis";
+    private static final String ATTR_COMMITTED_MILLIS = "committedMillis";
     private static final String ATTR_SESSION_STAGE_DIR = "sessionStageDir";
     private static final String ATTR_SESSION_STAGE_CID = "sessionStageCid";
     private static final String ATTR_PREPARED = "prepared";
@@ -291,6 +294,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
     @GuardedBy("mLock")
     private long updatedMillis;
 
+    /** Timestamp of the time this session is committed  */
+    @GuardedBy("mLock")
+    private long committedMillis;
+
     /** Uid of the creator of this session. */
     private final int mOriginalInstallerUid;
 
@@ -625,7 +632,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
             Context context, PackageManagerService pm,
             PackageSessionProvider sessionProvider, Looper looper, StagingManager stagingManager,
             int sessionId, int userId, int installerUid, @NonNull InstallSource installSource,
-            SessionParams params, long createdMillis,
+            SessionParams params, long createdMillis, long committedMillis,
             File stageDir, String stageCid, InstallationFile[] files,
             ArrayMap<String, List<CertifiedChecksum>> checksums,
             boolean prepared, boolean committed, boolean destroyed, boolean sealed,
@@ -648,6 +655,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         this.params = params;
         this.createdMillis = createdMillis;
         this.updatedMillis = createdMillis;
+        this.committedMillis = committedMillis;
         this.stageDir = stageDir;
         this.stageCid = stageCid;
         this.mShouldBeSealed = sealed;
@@ -1641,6 +1649,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                 mActiveCount.incrementAndGet();
 
                 mCommitted = true;
+                committedMillis = System.currentTimeMillis();
             }
             return true;
         } catch (PackageManagerException e) {
@@ -2963,7 +2972,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
     /**
      * @return the package name of this session
      */
-    String getPackageName() {
+    @VisibleForTesting(visibility = PACKAGE)
+    public String getPackageName() {
         synchronized (mLock) {
             return mPackageName;
         }
@@ -2978,6 +2988,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         }
     }
 
+    long getCommittedMillis() {
+        synchronized (mLock) {
+            return committedMillis;
+        }
+    }
+
     String getInstallerPackageName() {
         return getInstallSource().installerPackageName;
     }
@@ -3923,6 +3939,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         pw.printPair("mInstallerUid", mInstallerUid);
         pw.printPair("createdMillis", createdMillis);
         pw.printPair("updatedMillis", updatedMillis);
+        pw.printPair("committedMillis", committedMillis);
         pw.printPair("stageDir", stageDir);
         pw.printPair("stageCid", stageCid);
         pw.println();
@@ -4099,6 +4116,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                     mInstallSource.originatingPackageName);
             out.attributeLong(null, ATTR_CREATED_MILLIS, createdMillis);
             out.attributeLong(null, ATTR_UPDATED_MILLIS, updatedMillis);
+            out.attributeLong(null, ATTR_COMMITTED_MILLIS, committedMillis);
             if (stageDir != null) {
                 writeStringAttribute(out, ATTR_SESSION_STAGE_DIR,
                         stageDir.getAbsolutePath());
@@ -4252,6 +4270,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                 readStringAttribute(in, ATTR_ORIGINATING_PACKAGE_NAME);
         final long createdMillis = in.getAttributeLong(null, ATTR_CREATED_MILLIS);
         long updatedMillis = in.getAttributeLong(null, ATTR_UPDATED_MILLIS);
+        final long committedMillis = in.getAttributeLong(null, ATTR_COMMITTED_MILLIS, 0L);
         final String stageDirRaw = readStringAttribute(in, ATTR_SESSION_STAGE_DIR);
         final File stageDir = (stageDirRaw != null) ? new File(stageDirRaw) : null;
         final String stageCid = readStringAttribute(in, ATTR_SESSION_STAGE_CID);
@@ -4395,8 +4414,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                 installOriginatingPackageName, installerPackageName, installerAttributionTag);
         return new PackageInstallerSession(callback, context, pm, sessionProvider,
                 installerThread, stagingManager, sessionId, userId, installerUid,
-                installSource, params, createdMillis, stageDir, stageCid, fileArray, checksums,
-                prepared, committed, destroyed, sealed, childSessionIdsArray, parentSessionId,
-                isReady, isFailed, isApplied, stagedSessionErrorCode, stagedSessionErrorMessage);
+                installSource, params, createdMillis, committedMillis, stageDir, stageCid,
+                fileArray, checksums, prepared, committed, destroyed, sealed, childSessionIdsArray,
+                parentSessionId, isReady, isFailed, isApplied, stagedSessionErrorCode,
+                stagedSessionErrorMessage);
     }
 }
diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java
index 85c4ab2c8a10..222874d3eb82 100644
--- a/services/core/java/com/android/server/pm/StagingManager.java
+++ b/services/core/java/com/android/server/pm/StagingManager.java
@@ -59,6 +59,7 @@ import android.util.SparseArray;
 import android.util.apk.ApkSignatureVerifier;
 
 import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.content.PackageHelper;
 import com.android.internal.os.BackgroundThread;
 import com.android.server.LocalServices;
@@ -720,7 +721,8 @@ public class StagingManager {
      * </ul>
      * @throws PackageManagerException if session fails the check
      */
-    private void checkNonOverlappingWithStagedSessions(@NonNull PackageInstallerSession session)
+    @VisibleForTesting
+    void checkNonOverlappingWithStagedSessions(@NonNull PackageInstallerSession session)
             throws PackageManagerException {
         if (session.isMultiPackage()) {
             // We cannot say a parent session overlaps until we process its children
@@ -747,6 +749,13 @@ public class StagingManager {
                     continue;
                 }
 
+                if (stagedSession.getCommittedMillis() > session.getCommittedMillis()) {
+                    // Ignore sessions that are committed after the provided session. When there are
+                    // overlaps between sessions, we will fail the one committed later instead of
+                    // the earlier one.
+                    continue;
+                }
+
                 // From here on, stagedSession is a parent active staged session
 
                 // Check if session is one of the active sessions
@@ -793,7 +802,8 @@ public class StagingManager {
         }
     }
 
-    private void createSession(@NonNull PackageInstallerSession sessionInfo) {
+    @VisibleForTesting
+    void createSession(@NonNull PackageInstallerSession sessionInfo) {
         synchronized (mStagedSessions) {
             mStagedSessions.append(sessionInfo.sessionId, sessionInfo);
         }
diff --git a/services/tests/servicestests/src/com/android/server/pm/PackageInstallerSessionTest.java b/services/tests/servicestests/src/com/android/server/pm/PackageInstallerSessionTest.java
index 11d00f0fd406..5719dd2b0288 100644
--- a/services/tests/servicestests/src/com/android/server/pm/PackageInstallerSessionTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/PackageInstallerSessionTest.java
@@ -173,6 +173,7 @@ public class PackageInstallerSessionTest {
                 /* installSource */ installSource,
                 /* sessionParams */ params,
                 /* createdMillis */ 0L,
+                /* committedMillis */ 0L,
                 /* stageDir */ mTmpDir,
                 /* stageCid */ null,
                 /* files */ null,
diff --git a/services/tests/servicestests/src/com/android/server/pm/StagingManagerTest.java b/services/tests/servicestests/src/com/android/server/pm/StagingManagerTest.java
new file mode 100644
index 000000000000..4870c9e070cd
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/pm/StagingManagerTest.java
@@ -0,0 +1,128 @@
+/*
+ * Copyright (C) 2020 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;
+
+import android.content.Context;
+import android.content.pm.PackageInstaller;
+import android.os.storage.StorageManager;
+import android.platform.test.annotations.Presubmit;
+
+import com.android.internal.os.BackgroundThread;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;
+
+import java.io.File;
+
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertThrows;
+
+@Presubmit
+@RunWith(JUnit4.class)
+public class StagingManagerTest {
+    @Rule
+    public TemporaryFolder mTemporaryFolder = new TemporaryFolder();
+
+    private File mTmpDir;
+    private StagingManager mStagingManager;
+
+    @Before
+    public void setup() throws Exception {
+        MockitoAnnotations.initMocks(this);
+        StorageManager storageManager = Mockito.mock(StorageManager.class);
+        Context context = Mockito.mock(Context.class);
+        when(storageManager.isCheckpointSupported()).thenReturn(true);
+        when(context.getSystemService(eq(Context.POWER_SERVICE))).thenReturn(null);
+        when(context.getSystemService(eq(Context.STORAGE_SERVICE))).thenReturn(storageManager);
+
+        mTmpDir = mTemporaryFolder.newFolder("StagingManagerTest");
+        mStagingManager = new StagingManager(context, null);
+    }
+
+    /**
+     * Tests that sessions committed later shouldn't cause earlier ones to fail the overlapping
+     * check.
+     */
+    @Test
+    public void checkNonOverlappingWithStagedSessions_laterSessionShouldNotFailEarlierOnes()
+            throws Exception {
+        // Create 2 sessions with overlapping packages
+        PackageInstallerSession session1 = createSession(111, "com.foo", 1);
+        PackageInstallerSession session2 = createSession(222, "com.foo", 2);
+
+        mStagingManager.createSession(session1);
+        mStagingManager.createSession(session2);
+        // Session1 should not fail in spite of the overlapping packages
+        mStagingManager.checkNonOverlappingWithStagedSessions(session1);
+        // Session2 should fail due to overlapping packages
+        assertThrows(PackageManagerException.class,
+                () -> mStagingManager.checkNonOverlappingWithStagedSessions(session2));
+    }
+
+    private PackageInstallerSession createSession(int sessionId, String packageName,
+            long committedMillis) {
+        PackageInstaller.SessionParams params = new PackageInstaller.SessionParams(
+                PackageInstaller.SessionParams.MODE_FULL_INSTALL);
+        params.isStaged = true;
+
+        InstallSource installSource = InstallSource.create("testInstallInitiator",
+                "testInstallOriginator", "testInstaller", "testAttributionTag");
+
+        PackageInstallerSession session = new PackageInstallerSession(
+                /* callback */ null,
+                /* context */ null,
+                /* pm */ null,
+                /* sessionProvider */ null,
+                /* looper */ BackgroundThread.getHandler().getLooper(),
+                /* stagingManager */ null,
+                /* sessionId */ sessionId,
+                /* userId */ 456,
+                /* installerUid */ -1,
+                /* installSource */ installSource,
+                /* sessionParams */ params,
+                /* createdMillis */ 0L,
+                /* committedMillis */ committedMillis,
+                /* stageDir */ mTmpDir,
+                /* stageCid */ null,
+                /* files */ null,
+                /* checksums */ null,
+                /* prepared */ true,
+                /* committed */ true,
+                /* destroyed */ false,
+                /* sealed */ false,  // Setting to true would trigger some PM logic.
+                /* childSessionIds */ null,
+                /* parentSessionId */ -1,
+                /* isReady */ false,
+                /* isFailed */ false,
+                /* isApplied */false,
+                /* stagedSessionErrorCode */ PackageInstaller.SessionInfo.STAGED_SESSION_NO_ERROR,
+                /* stagedSessionErrorMessage */ "no error");
+
+        session = spy(session);
+        doReturn(packageName).when(session).getPackageName();
+        return session;
+    }
+}