From 48955967478bbcec560d41ae9c21465798d98dd8 Mon Sep 17 00:00:00 2001 From: Mohammad Samiul Islam Date: Tue, 7 Jul 2020 19:29:11 +0100 Subject: [PATCH] Stop creating extra apk session during pre-reboot verification Now that verification logic has been separated out from install flow, we can reuse the original session for apk verification without depending on extra session with dry run flag. Apex sessions are also allowed to follow along but they are ignored in the verification process for now. Unused codes will be removed in follow up cls. Bug: 159336213 Test: atest StagedInstallTest Test: atest GtsSecurityHostTestCases Test: atest CtsAtomicInstallTestCases Change-Id: I8b0511a6af4360253595bacc6c9f72da81920246 --- .../server/pm/PackageInstallerSession.java | 61 +++++++++++++++---- .../server/pm/PackageManagerService.java | 7 +++ .../com/android/server/pm/StagingManager.java | 31 ++++++---- 3 files changed, 75 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 28c5e964fe275..ec96789e24fb3 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -134,6 +134,7 @@ import com.android.internal.os.SomeArgs; import com.android.internal.util.ArrayUtils; import com.android.internal.util.FrameworkStatsLog; import com.android.internal.util.IndentingPrintWriter; +import com.android.internal.util.Preconditions; import com.android.server.LocalServices; import com.android.server.pm.Installer.InstallerException; import com.android.server.pm.dex.DexManager; @@ -1554,12 +1555,28 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } private void onSessionValidationFailure(int error, String detailMessage) { - // Session is sealed but could not be verified, we need to destroy it. + // Session is sealed but could not be validated, we need to destroy it. destroyInternal(); // Dispatch message to remove session from PackageInstallerService. dispatchSessionFinished(error, detailMessage, null); } + private void onSessionVerificationFailure(int error, String detailMessage) { + Slog.e(TAG, "Failed to verify session " + sessionId + " [" + detailMessage + "]"); + // Session is sealed and committed but could not be verified, we need to destroy it. + destroyInternal(); + if (isStaged()) { + setStagedSessionFailed( + SessionInfo.STAGED_SESSION_VERIFICATION_FAILED, detailMessage); + // TODO(b/136257624): Remove this once all verification logic has been transferred out + // of StagingManager. + mStagingManager.notifyVerificationComplete(sessionId); + } else { + // Dispatch message to remove session from PackageInstallerService. + dispatchSessionFinished(error, detailMessage, null); + } + } + private void onStorageUnhealthy() { final String packageName = getPackageName(); if (TextUtils.isEmpty(packageName)) { @@ -1680,7 +1697,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } if (params.isStaged) { mStagingManager.commitSession(this); - destroyInternal(); + // TODO(b/136257624): CTS test fails if we don't send session finished broadcast, even + // though ideally, we just need to send session committed broadcast. dispatchSessionFinished(PackageManager.INSTALL_SUCCEEDED, "Session staged", null); return; } @@ -1691,14 +1709,30 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { "APEX packages can only be installed using staged sessions.", null); return; } + verify(); + } + /** + * Resumes verification process for non-final committed staged session. + * + * Useful if a device gets rebooted before verification is complete and we need to restart the + * verification. + */ + void verifyStagedSession() { + assertCallerIsOwnerOrRootOrSystemLocked(); + Preconditions.checkArgument(isCommitted()); + Preconditions.checkArgument(isStaged()); + Preconditions.checkArgument(!mStagedSessionApplied && !mStagedSessionFailed); + + verify(); + } + + private void verify() { try { verifyNonStaged(); } catch (PackageManagerException e) { final String completeMsg = ExceptionUtils.getCompleteMessage(e); - Slog.e(TAG, "Commit of session " + sessionId + " failed: " + completeMsg); - destroyInternal(); - dispatchSessionFinished(e.error, completeMsg, null); + onSessionVerificationFailure(e.error, completeMsg); } } @@ -1846,7 +1880,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @GuardedBy("mLock") private PackageManagerService.VerificationParams makeVerificationParamsLocked() throws PackageManagerException { - if (!params.isMultiPackage) { + // TODO(b/136257624): Some logic in this if block probably belongs in + // makeInstallParams(). + if (!params.isMultiPackage && !isApexInstallation()) { Objects.requireNonNull(mPackageName); Objects.requireNonNull(mSigningDetails); Objects.requireNonNull(mResolvedBaseFile); @@ -1923,8 +1959,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { if (returnCode == PackageManager.INSTALL_SUCCEEDED) { onVerificationComplete(); } else { - destroyInternal(); - dispatchSessionFinished(returnCode, msg, extras); + onSessionVerificationFailure(returnCode, msg); } } }; @@ -1946,9 +1981,13 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } private void onVerificationComplete() { - if ((params.installFlags & PackageManager.INSTALL_DRY_RUN) != 0) { - destroyInternal(); - dispatchSessionFinished(PackageManager.INSTALL_SUCCEEDED, "Dry run", new Bundle()); + // Staged sessions will be installed later during boot + if (isStaged()) { + // TODO(b/136257624): Remove this once all verification logic has been transferred out + // of StagingManager. + mStagingManager.notifyPreRebootVerification_Apk_Complete(sessionId); + // TODO(b/136257624): We also need to destroy internals for verified staged session, + // otherwise file descriptors are never closed for verified staged session until reboot return; } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 5fba8b93ff4c2..266ec866f4c26 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -15198,6 +15198,13 @@ public class PackageManagerService extends IPackageManager.Stub } public void handleStartCopy() { + if ((installFlags & PackageManager.INSTALL_APEX) != 0) { + // Apex packages get verified in StagingManager currently. + // TODO(b/136257624): Move apex verification logic out of StagingManager + mRet = INSTALL_SUCCEEDED; + return; + } + PackageInfoLite pkgLite = PackageManagerServiceUtils.getMinimalPackageInfo(mContext, origin.resolvedPath, installFlags, packageAbiOverride); diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 6d80f08ecbf66..981006e4822a8 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -1299,6 +1299,20 @@ public class StagingManager { return session; } + // TODO(b/136257624): Temporary API to let PMS communicate with StagingManager. When all + // verification logic is extraced out of StagingManager into PMS, we can remove + // this. + void notifyVerificationComplete(int sessionId) { + mPreRebootVerificationHandler.onPreRebootVerificationComplete(sessionId); + } + + // TODO(b/136257624): Temporary API to let PMS communicate with StagingManager. When all + // verification logic is extraced out of StagingManager into PMS, we can remove + // this. + void notifyPreRebootVerification_Apk_Complete(int sessionId) { + mPreRebootVerificationHandler.notifyPreRebootVerification_Apk_Complete(sessionId); + } + private final class PreRebootVerificationHandler extends Handler { // Hold session ids before handler gets ready to do the verification. private IntArray mPendingSessionIds; @@ -1500,25 +1514,16 @@ public class StagingManager { } /** - * Pre-reboot verification state for apk files: - *

+ * Pre-reboot verification state for apk files. Session is sent to + * {@link PackageManagerService} for verification and it notifies back the result via + * {@link #notifyPreRebootVerification_Apk_Complete(int)} */ private void handlePreRebootVerification_Apk(@NonNull PackageInstallerSession session) { if (!sessionContainsApk(session)) { notifyPreRebootVerification_Apk_Complete(session.sessionId); return; } - - try { - Slog.d(TAG, "Running a pre-reboot verification for APKs in session " - + session.sessionId + " by performing a dry-run install"); - // verifyApksInSession will notify the handler when APK verification is complete - verifyApksInSession(session); - } catch (PackageManagerException e) { - onPreRebootVerificationFailure(session, e.error, e.getMessage()); - } + session.verifyStagedSession(); } private void verifyApksInSession(PackageInstallerSession session)