From 5d959629c7553733ca45a12867db9ab4eb7a10fc Mon Sep 17 00:00:00 2001 From: JW Wang Date: Thu, 25 Feb 2021 11:55:39 +0800 Subject: [PATCH 1/3] Do APK verification before committing sessions to StagingManager (1/n) This allows us to share the verification flow between both staged and non-staged sessions. * Do APK verification before committing sessions to StagingManager. * remove handlePreRebootVerification_Apk() and its friends which are dead code now. This fixes TODOs and reduces coupling between StagingManager and PackageInstallerSession. Bug: 181092466 Test: atest StagedInstallTest StagedInstallInternalTest Change-Id: I4f9f16471b8e321e1ded5ba6dc9d1cfb982e6bba --- .../server/pm/PackageInstallerSession.java | 49 +++++++----------- .../com/android/server/pm/StagingManager.java | 51 ++----------------- 2 files changed, 24 insertions(+), 76 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 903652ab76a59..fcebffaaad35e 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -699,28 +699,22 @@ 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. + * Called when pre-reboot verification is about to start. This shouldn't be called + * on a destroyed session. */ - @Override - public boolean notifyStartPreRebootVerification() { + private void notifyStartPreRebootVerification() { synchronized (mLock) { + Preconditions.checkState(!mDestroyed); if (mInPreRebootVerification) { throw new IllegalStateException("Pre-reboot verification has started"); } - if (mDestroyed) { - return false; - } mInPreRebootVerification = true; - return true; } } /** - * 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. + * Notified by the staging manager or PIS that pre-reboot verification has ended. + * Now it is safe to clean up the session if {@link #abandon()} has been called previously. */ @Override public void notifyEndPreRebootVerification() { @@ -744,6 +738,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { assertCallerIsOwnerOrRootOrSystemLocked(); Preconditions.checkArgument(isCommitted()); Preconditions.checkArgument(!mSessionApplied && !mSessionFailed); + notifyStartPreRebootVerification(); verify(); } @@ -2037,9 +2032,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { if (isStaged()) { mStagedSession.setSessionFailed( SessionInfo.STAGED_SESSION_VERIFICATION_FAILED, msgWithErrorCode); - // TODO(b/136257624): Remove this once all verification logic has been transferred out - // of StagingManager. - mStagingManager.notifyVerificationComplete(mStagedSession); + mStagedSession.notifyEndPreRebootVerification(); } else { // Dispatch message to remove session from PackageInstallerService. dispatchSessionFinished(error, msg, null); @@ -2159,24 +2152,19 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { .write(); } if (params.isStaged) { - mStagingManager.commitSession(mStagedSession); // 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(INSTALL_SUCCEEDED, "Session staged", null); - return; - } - if (isApexSession()) { - destroyInternal(); - dispatchSessionFinished(PackageManager.INSTALL_FAILED_INTERNAL_ERROR, - "APEX packages can only be installed using staged sessions.", null); - return; + mStagedSession.verifySession(); + } else { + verify(); } - verify(); } private void verify() { try { + //TODO: re-run verify() if reboot happens before staged sessions reach READY verifyNonStaged(); } catch (PackageManagerException e) { final String completeMsg = ExceptionUtils.getCompleteMessage(e); @@ -2451,16 +2439,17 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } private void onVerificationComplete() { - // Staged sessions will be installed later during boot + // APK verification is done. Continue the installation depending on whether it is a + // staged session or not. For a staged session, we will hand it over to the staging + // manager to complete the installation. if (isStaged()) { - // TODO(b/136257624): Remove this once all verification logic has been transferred out - // of StagingManager. - mStagingManager.notifyPreRebootVerification_Apk_Complete(mStagedSession); - // 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 + mStagingManager.commitSession(mStagedSession); return; } + // APEX sessions should be handled above + Preconditions.checkState(!isApexSession()); + install(); } diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 0a74032ab214a..2bbb9c5a6ef76 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -136,7 +136,6 @@ public class StagingManager { boolean hasParentSessionId(); long getCommittedMillis(); void abandon(); - boolean notifyStartPreRebootVerification(); void notifyEndPreRebootVerification(); void verifySession(); } @@ -1083,20 +1082,6 @@ public class StagingManager { return session; } - // 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(StagedSession session) { - mPreRebootVerificationHandler.onPreRebootVerificationComplete(session); - } - - // 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 notifyPreRebootVerification_Apk_Complete(@NonNull StagedSession session) { - mPreRebootVerificationHandler.notifyPreRebootVerification_Apk_Complete(session); - } - private final class PreRebootVerificationHandler extends Handler { // Hold sessions before handler gets ready to do the verification. private List mPendingSessions; @@ -1114,7 +1099,6 @@ public class StagingManager { *

* @@ -1122,8 +1106,7 @@ public class StagingManager { */ private static final int MSG_PRE_REBOOT_VERIFICATION_START = 1; private static final int MSG_PRE_REBOOT_VERIFICATION_APEX = 2; - private static final int MSG_PRE_REBOOT_VERIFICATION_APK = 3; - private static final int MSG_PRE_REBOOT_VERIFICATION_END = 4; + private static final int MSG_PRE_REBOOT_VERIFICATION_END = 3; @Override public void handleMessage(Message msg) { @@ -1143,9 +1126,6 @@ public class StagingManager { case MSG_PRE_REBOOT_VERIFICATION_APEX: handlePreRebootVerification_Apex(session, rollbackId); break; - case MSG_PRE_REBOOT_VERIFICATION_APK: - handlePreRebootVerification_Apk(session); - break; case MSG_PRE_REBOOT_VERIFICATION_END: handlePreRebootVerification_End(session); break; @@ -1181,12 +1161,10 @@ public class StagingManager { return; } - if (session.notifyStartPreRebootVerification()) { - int sessionId = session.sessionId(); - Slog.d(TAG, "Starting preRebootVerification for session " + sessionId); - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, -1, session) - .sendToTarget(); - } + int sessionId = session.sessionId(); + Slog.d(TAG, "Starting preRebootVerification for session " + sessionId); + obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, -1, session) + .sendToTarget(); } private void onPreRebootVerificationFailure(StagedSession session, @@ -1215,12 +1193,6 @@ public class StagingManager { private void notifyPreRebootVerification_Apex_Complete( @NonNull StagedSession session) { - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APK, session.sessionId(), -1, session) - .sendToTarget(); - } - - private void notifyPreRebootVerification_Apk_Complete( - @NonNull StagedSession session) { obtainMessage(MSG_PRE_REBOOT_VERIFICATION_END, session.sessionId(), -1, session) .sendToTarget(); } @@ -1307,19 +1279,6 @@ public class StagingManager { notifyPreRebootVerification_Apex_Complete(session); } - /** - * 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} - */ - private void handlePreRebootVerification_Apk(@NonNull StagedSession session) { - if (!session.containsApkSession()) { - notifyPreRebootVerification_Apk_Complete(session); - return; - } - session.verifySession(); - } - /** * Pre-reboot verification state for wrapping up: *