Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c640ee7f authored by JW Wang's avatar JW Wang
Browse files

Sessions committed later shouldn't cause earlier ones to fail

When 2 sessions have overlapping packages, the one committed earlier
should pass the overlapping check while the later one should fail.

1. Add a field to denote the time when a session is committed.
2. When doing overlapping check, only consider sessions that are
   committed before the current session.

Bug: 174442862
Test: atest StagingManagerTest PackageInstallerSessionTest

Change-Id: Ieeafc879f32b1228d451307b194d65146c7496c1
parent c43d935c
Loading
Loading
Loading
Loading
+2 −2
Original line number Diff line number Diff line
@@ -738,8 +738,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) {
+25 −5
Original line number Diff line number Diff line
@@ -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;
@@ -1640,6 +1648,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                mActiveCount.incrementAndGet();

                mCommitted = true;
                committedMillis = System.currentTimeMillis();
            }
            return true;
        } catch (PackageManagerException e) {
@@ -2962,7 +2971,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;
        }
@@ -2977,6 +2987,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
        }
    }

    long getCommittedMillis() {
        synchronized (mLock) {
            return committedMillis;
        }
    }

    String getInstallerPackageName() {
        return getInstallSource().installerPackageName;
    }
@@ -3922,6 +3938,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();
@@ -4098,6 +4115,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());
@@ -4251,6 +4269,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);
@@ -4394,8 +4413,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);
    }
}
+12 −2
Original line number Diff line number Diff line
@@ -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);
        }
+1 −0
Original line number Diff line number Diff line
@@ -173,6 +173,7 @@ public class PackageInstallerSessionTest {
                /* installSource */ installSource,
                /* sessionParams */ params,
                /* createdMillis */ 0L,
                /* committedMillis */ 0L,
                /* stageDir */ mTmpDir,
                /* stageCid */ null,
                /* files */ null,
+128 −0
Original line number Diff line number Diff line
/*
 * 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;
    }
}