diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 51164ba412b3f..515b90913870e 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -249,6 +249,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { private final PackageManagerService mPm; private final Handler mHandler; private final PackageSessionProvider mSessionProvider; + /** + * Note all calls must be done outside {@link #mLock} to prevent lock inversion. + */ private final StagingManager mStagingManager; final int sessionId; @@ -388,6 +391,20 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @GuardedBy("mLock") private String mStagedSessionErrorMessage; + /** + * The callback to run when pre-reboot verification has ended. Used by {@link #abandonStaged()} + * to delay session clean-up until it is safe to do so. + */ + @GuardedBy("mLock") + @Nullable + private Runnable mPendingAbandonCallback; + /** + * {@code true} if pre-reboot verification is ongoing which means it is not safe for + * {@link #abandon()} to clean up staging directories. + */ + @GuardedBy("mLock") + private boolean mInPreRebootVerification; + /** * Path to the validated base APK for this session, which may point at an * APK inside the session (when the session defines the base), or it may @@ -1454,26 +1471,54 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { // TODO(patb): since the work done here for a parent session in a multi-package install is // mostly superficial, consider splitting this method for the parent and // single / child sessions. - synchronized (mLock) { - if (mCommitted) { - return true; + try { + synchronized (mLock) { + if (mCommitted) { + return true; + } + // Read transfers from the original owner stay open, but as the session's data + // cannot be modified anymore, there is no leak of information. For staged sessions, + // further validation is performed by the staging manager. + if (!params.isMultiPackage) { + if (!prepareDataLoaderLocked()) { + return false; + } + + if (isApexInstallation()) { + validateApexInstallLocked(); + } else { + validateApkInstallLocked(); + } + } } - if (!streamAndValidateLocked()) { - return false; + if (params.isStaged) { + mStagingManager.checkNonOverlappingWithStagedSessions(this); } - // Client staging is fully done at this point - mClientProgress = 1f; - computeProgressLocked(true); + synchronized (mLock) { + if (mDestroyed) { + throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, + "Session destroyed"); + } + // Client staging is fully done at this point + mClientProgress = 1f; + computeProgressLocked(true); - // This ongoing commit should keep session active, even though client - // will probably close their end. - mActiveCount.incrementAndGet(); + // This ongoing commit should keep session active, even though client + // will probably close their end. + mActiveCount.incrementAndGet(); - mCommitted = true; + mCommitted = true; + } + return true; + } 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. + throw onSessionValidationFailure(new PackageManagerException(e)); } - return true; } @GuardedBy("mLock") @@ -1511,44 +1556,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } - /** - * Prepare DataLoader and stream content for DataLoader sessions. - * Validate the contents of all session. - * - * @return false if the data loader could not be prepared. - * @throws PackageManagerException when an unrecoverable exception is encountered - */ - @GuardedBy("mLock") - private boolean streamAndValidateLocked() throws PackageManagerException { - try { - // Read transfers from the original owner stay open, but as the session's data cannot - // be modified anymore, there is no leak of information. For staged sessions, further - // validation is performed by the staging manager. - if (!params.isMultiPackage) { - if (!prepareDataLoaderLocked()) { - return false; - } - - if (isApexInstallation()) { - validateApexInstallLocked(); - } else { - validateApkInstallLocked(); - } - } - - if (params.isStaged) { - mStagingManager.checkNonOverlappingWithStagedSessions(this); - } - return true; - } 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. - throw onSessionValidationFailure(new PackageManagerException(e)); - } - } - private PackageManagerException onSessionValidationFailure(PackageManagerException e) { onSessionValidationFailure(e.error, ExceptionUtils.getCompleteMessage(e)); return e; @@ -1571,7 +1578,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { SessionInfo.STAGED_SESSION_VERIFICATION_FAILED, msgWithErrorCode); // TODO(b/136257624): Remove this once all verification logic has been transferred out // of StagingManager. - mStagingManager.notifyVerificationComplete(sessionId); + mStagingManager.notifyVerificationComplete(this); } else { // Dispatch message to remove session from PackageInstallerService. dispatchSessionFinished(error, msg, null); @@ -1837,21 +1844,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { throws PackageManagerException { assertNotLocked("makeSessionActive"); - synchronized (mLock) { - if (mRelinquished) { - throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, - "Session relinquished"); - } - if (mDestroyed) { - throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, - "Session destroyed"); - } - if (!mSealed) { - throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, - "Session not sealed"); - } - } - // TODO(b/159331446): Move this to makeSessionActiveForInstall and update javadoc if (!params.isMultiPackage && needToAskForPermissions()) { // User needs to confirm installation; @@ -1881,6 +1873,19 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @GuardedBy("mLock") private PackageManagerService.VerificationParams makeVerificationParamsLocked() throws PackageManagerException { + if (mRelinquished) { + throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, + "Session relinquished"); + } + if (mDestroyed) { + throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, + "Session destroyed"); + } + if (!mSealed) { + throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, + "Session not sealed"); + } + // TODO(b/136257624): Some logic in this if block probably belongs in // makeInstallParams(). if (!params.isMultiPackage && !isApexInstallation()) { @@ -2787,14 +2792,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } private void abandonStaged() { + final Runnable r; synchronized (mLock) { - if (mDestroyed) { - // If a user abandons staged session in an unsafe state, then system will try to - // abandon the destroyed staged session when it is safe on behalf of the user. - assertCallerIsOwnerOrRootOrSystemLocked(); - } else { - assertCallerIsOwnerOrRootLocked(); - } + assertCallerIsOwnerOrRootLocked(); if (isStagedAndInTerminalState()) { // We keep the session in the database if it's in a finalized state. It will be // removed by PackageInstallerService when the last update time is old enough. @@ -2803,17 +2803,25 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return; } mDestroyed = true; - if (mCommitted) { - if (!mStagingManager.abortCommittedSessionLocked(this)) { - // Do not clean up the staged session from system. It is not safe yet. - mCallback.onStagedSessionChanged(this); - return; + boolean isCommitted = mCommitted; + List childSessions = getChildSessionsLocked(); + r = () -> { + assertNotLocked("abandonStaged"); + if (isCommitted) { + mStagingManager.abortCommittedSession(this); } + cleanStageDir(childSessions); + destroyInternal(); + dispatchSessionFinished(INSTALL_FAILED_ABORTED, "Session was abandoned", null); + }; + if (mInPreRebootVerification) { + // Pre-reboot verification is ongoing. It is not safe to clean up the session yet. + mPendingAbandonCallback = r; + mCallback.onStagedSessionChanged(this); + return; } - cleanStageDir(getChildSessionsLocked()); - destroyInternal(); } - dispatchSessionFinished(INSTALL_FAILED_ABORTED, "Session was abandoned", null); + r.run(); } @Override @@ -2830,6 +2838,50 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } + /** + * Notified by the staging manager that pre-reboot verification is about to start. The return + * value should be checked to decide whether it is OK to start pre-reboot verification. In + * the case of a destroyed session, {@code false} is returned and there is no need to start + * pre-reboot verification. + */ + boolean notifyStagedStartPreRebootVerification() { + synchronized (mLock) { + if (mInPreRebootVerification) { + throw new IllegalStateException("Pre-reboot verification has started"); + } + if (mDestroyed) { + return false; + } + mInPreRebootVerification = true; + return true; + } + } + + private void dispatchPendingAbandonCallback() { + final Runnable callback; + synchronized (mLock) { + callback = mPendingAbandonCallback; + mPendingAbandonCallback = null; + } + if (callback != null) { + callback.run(); + } + } + + /** + * Notified by the staging manager that pre-reboot verification has ended. Now it is safe to + * clean up the session if {@link #abandon()} has been called previously. + */ + void notifyStagedEndPreRebootVerification() { + synchronized (mLock) { + if (!mInPreRebootVerification) { + throw new IllegalStateException("Pre-reboot verification not started"); + } + mInPreRebootVerification = false; + } + dispatchPendingAbandonCallback(); + } + @Override public boolean isMultiPackage() { return params.isMultiPackage; diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 0c4eaec32ba52..462b215353717 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -60,7 +60,6 @@ import android.util.ArraySet; import android.util.IntArray; import android.util.Slog; import android.util.SparseArray; -import android.util.SparseBooleanArray; import android.util.SparseIntArray; import android.util.apk.ApkSignatureVerifier; @@ -1028,22 +1027,12 @@ public class StagingManager { /** *

Abort committed staged session - * - *

This method must be called while holding {@link PackageInstallerSession#mLock}. - * - *

The method returns {@code false} to indicate it is not safe to clean up the session from - * system yet. When it is safe, the method returns {@code true}. - * - *

When it is safe to clean up, {@link StagingManager} will call - * {@link PackageInstallerSession#abandon()} on the session again. - * - * @return {@code true} if it is safe to cleanup the session resources, otherwise {@code false}. */ - boolean abortCommittedSessionLocked(@NonNull PackageInstallerSession session) { + void abortCommittedSession(@NonNull PackageInstallerSession session) { int sessionId = session.sessionId; - if (session.isStagedSessionApplied()) { - Slog.w(TAG, "Cannot abort applied session : " + sessionId); - return false; + if (session.isStagedAndInTerminalState()) { + Slog.w(TAG, "Cannot abort session in final state: " + sessionId); + return; } if (!session.isDestroyed()) { throw new IllegalStateException("Committed session must be destroyed before aborting it" @@ -1051,15 +1040,7 @@ public class StagingManager { } if (getStagedSession(sessionId) == null) { Slog.w(TAG, "Session " + sessionId + " has been abandoned already"); - return false; - } - - // If pre-reboot verification is running, then return false. StagingManager will call - // abandon again when pre-reboot verification ends. - if (mPreRebootVerificationHandler.isVerificationRunning(sessionId)) { - Slog.w(TAG, "Session " + sessionId + " aborted before pre-reboot " - + "verification completed."); - return false; + return; } // A session could be marked ready once its pre-reboot verification ends @@ -1075,7 +1056,6 @@ public class StagingManager { // Session was successfully aborted from apexd (if required) and pre-reboot verification // is also complete. It is now safe to clean up the session from system. abortSession(session); - return true; } /** @@ -1264,8 +1244,8 @@ public class StagingManager { // TODO(b/136257624): Temporary API to let PMS communicate with StagingManager. When all // verification logic is extracted out of StagingManager into PMS, we can remove // this. - void notifyVerificationComplete(int sessionId) { - mPreRebootVerificationHandler.onPreRebootVerificationComplete(sessionId); + void notifyVerificationComplete(PackageInstallerSession session) { + mPreRebootVerificationHandler.onPreRebootVerificationComplete(session); } // TODO(b/136257624): Temporary API to let PMS communicate with StagingManager. When all @@ -1279,8 +1259,6 @@ public class StagingManager { // Hold session ids before handler gets ready to do the verification. private IntArray mPendingSessionIds; private boolean mIsReady; - @GuardedBy("mVerificationRunning") - private final SparseBooleanArray mVerificationRunning = new SparseBooleanArray(); PreRebootVerificationHandler(Looper looper) { super(looper); @@ -1316,7 +1294,7 @@ public class StagingManager { } if (session.isDestroyed() || session.isStagedSessionFailed()) { // No point in running verification on a destroyed/failed session - onPreRebootVerificationComplete(sessionId); + onPreRebootVerificationComplete(session); return; } switch (msg.what) { @@ -1357,15 +1335,10 @@ public class StagingManager { } PackageInstallerSession session = getStagedSession(sessionId); - synchronized (mVerificationRunning) { - // Do not start verification on a session that has been abandoned - if (session == null || session.isDestroyed()) { - return; - } + if (session != null && session.notifyStagedStartPreRebootVerification()) { Slog.d(TAG, "Starting preRebootVerification for session " + sessionId); - mVerificationRunning.put(sessionId, true); + obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, 0).sendToTarget(); } - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, 0).sendToTarget(); } private void onPreRebootVerificationFailure(PackageInstallerSession session, @@ -1376,28 +1349,14 @@ public class StagingManager { // failed on next step and staging directory for session will be deleted. } session.setStagedSessionFailed(errorCode, errorMessage); - onPreRebootVerificationComplete(session.sessionId); + onPreRebootVerificationComplete(session); } // Things to do when pre-reboot verification completes for a particular sessionId - private void onPreRebootVerificationComplete(int sessionId) { - // Remove it from mVerificationRunning so that verification is considered complete - synchronized (mVerificationRunning) { - Slog.d(TAG, "Stopping preRebootVerification for session " + sessionId); - mVerificationRunning.delete(sessionId); - } - // Check if the session was destroyed while pre-reboot verification was running. If so, - // abandon it again. - PackageInstallerSession session = getStagedSession(sessionId); - if (session != null && session.isDestroyed()) { - session.abandon(); - } - } - - private boolean isVerificationRunning(int sessionId) { - synchronized (mVerificationRunning) { - return mVerificationRunning.get(sessionId); - } + private void onPreRebootVerificationComplete(PackageInstallerSession session) { + int sessionId = session.sessionId; + Slog.d(TAG, "Stopping preRebootVerification for session " + sessionId); + session.notifyStagedEndPreRebootVerification(); } private void notifyPreRebootVerification_Start_Complete(int sessionId) { @@ -1516,7 +1475,7 @@ public class StagingManager { // or activate its apex, there won't be any files to work with as they will be cleaned // up by the system as part of abandonment. If session is abandoned before this point, // then the session is already destroyed and cannot be marked ready anymore. - onPreRebootVerificationComplete(session.sessionId); + onPreRebootVerificationComplete(session); // Proactively mark session as ready before calling apexd. Although this call order // looks counter-intuitive, this is the easiest way to ensure that session won't end up