From fdf525f512be2832f1a5ee2aad3e35a9872eedf5 Mon Sep 17 00:00:00 2001 From: Mohammad Samiul Islam Date: Fri, 23 Oct 2020 13:53:52 +0100 Subject: [PATCH 1/3] Prevent exceptions in pre-reboot verification from crashing system server An unhandled exception during pre-reboot verification will cause system server to crash and restart. Instead of allowing the exception to crash the system server, we now catch it and fail the corresponding staged session with appropriate message. Bug: 170784748 Test: manual Test: verified that without this fix, any unhandled exception crashes the system server. Change-Id: If100796687906be67bc02ea63a78e1b6e291ce7a Merged-In: If100796687906be67bc02ea63a78e1b6e291ce7a (cherry picked from commit 5a2548589560404f9fce4914216564b0333895d4) --- .../com/android/server/pm/StagingManager.java | 64 +++++++++++++++---- 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 600357b7697fc..88e86763a4637 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -1072,6 +1072,26 @@ public class StagingManager { return true; } + /** + * Ensure that there is no active apex session staged in apexd for the given session. + * + * @return returns true if it is ensured that there is no active apex session, otherwise false + */ + private boolean ensureActiveApexSessionIsAborted(PackageInstallerSession session) { + if (!sessionContainsApex(session)) { + return true; + } + final ApexSessionInfo apexSession = mApexManager.getStagedSessionInfo(session.sessionId); + if (apexSession == null || isApexSessionFinalized(apexSession)) { + return true; + } + try { + return mApexManager.abortStagedSession(session.sessionId); + } catch (PackageManagerException ignore) { + return false; + } + } + private boolean isApexSessionFinalized(ApexSessionInfo session) { /* checking if the session is in a final state, i.e., not active anymore */ return session.isUnknown || session.isActivationFailed || session.isSuccess @@ -1303,19 +1323,26 @@ public class StagingManager { onPreRebootVerificationComplete(sessionId); return; } - switch (msg.what) { - case MSG_PRE_REBOOT_VERIFICATION_START: - handlePreRebootVerification_Start(session); - break; - case MSG_PRE_REBOOT_VERIFICATION_APEX: - handlePreRebootVerification_Apex(session); - break; - case MSG_PRE_REBOOT_VERIFICATION_APK: - handlePreRebootVerification_Apk(session); - break; - case MSG_PRE_REBOOT_VERIFICATION_END: - handlePreRebootVerification_End(session); - break; + try { + switch (msg.what) { + case MSG_PRE_REBOOT_VERIFICATION_START: + handlePreRebootVerification_Start(session); + break; + case MSG_PRE_REBOOT_VERIFICATION_APEX: + handlePreRebootVerification_Apex(session); + break; + case MSG_PRE_REBOOT_VERIFICATION_APK: + handlePreRebootVerification_Apk(session); + break; + case MSG_PRE_REBOOT_VERIFICATION_END: + handlePreRebootVerification_End(session); + break; + } + } catch (Exception e) { + Slog.e(TAG, "Pre-reboot verification failed due to unhandled exception", e); + onPreRebootVerificationFailure(session, + SessionInfo.STAGED_SESSION_ACTIVATION_FAILED, + "Pre-reboot verification failed due to unhandled exception: " + e); } } @@ -1352,6 +1379,17 @@ public class StagingManager { obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, 0).sendToTarget(); } + private void onPreRebootVerificationFailure(PackageInstallerSession session, + @SessionInfo.StagedSessionErrorCode int errorCode, String errorMessage) { + if (!ensureActiveApexSessionIsAborted(session)) { + Slog.e(TAG, "Failed to abort apex session " + session.sessionId); + // Safe to ignore active apex session abortion failure since session will be marked + // failed on next step and staging directory for session will be deleted. + } + session.setStagedSessionFailed(errorCode, errorMessage); + onPreRebootVerificationComplete(session.sessionId); + } + // 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 From ed2cd670ec7974229f216b8d8f02cd9981fba350 Mon Sep 17 00:00:00 2001 From: Mohammad Samiul Islam Date: Fri, 23 Oct 2020 14:24:26 +0100 Subject: [PATCH 2/3] Prevent exceptions during staged install from crashing system server An unhandled exception during staged install at boot time will cause system server to crash and restart. Upon restart, staged install will resume again and it will cause system server to crash again. Thus creating a loop. Instead of allowing the exception to crash the system server, we now catch it and fail the corresponding staged session with appropriate message. Bug: 170784748 Test: manual Test: verified that without this fix, any unhandled exception sends system server into a crash loop Change-Id: I9abec5d2401af95ecb095fa3c45960d2f15d4e74 Merged-In: I9abec5d2401af95ecb095fa3c45960d2f15d4e74 (cherry picked from commit 1c3e99b6c5a13ec2d7a8d3e618ce459fec0de7b7) --- .../com/android/server/pm/StagingManager.java | 87 ++++++++++--------- 1 file changed, 45 insertions(+), 42 deletions(-) diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 88e86763a4637..f779b6e2cc82f 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -564,7 +564,8 @@ public class StagingManager { } } - private void resumeSession(@NonNull PackageInstallerSession session) { + private void resumeSession(@NonNull PackageInstallerSession session) + throws PackageManagerException { Slog.d(TAG, "Resuming session " + session.sessionId); final boolean hasApex = sessionContainsApex(session); @@ -628,10 +629,8 @@ public class StagingManager { if (apexSessionInfo == null) { final String errorMsg = "apexd did not know anything about a staged session " + "supposed to be activated"; - session.setStagedSessionFailed(SessionInfo.STAGED_SESSION_ACTIVATION_FAILED, - errorMsg); - abortCheckpoint(session.sessionId, errorMsg); - return; + throw new PackageManagerException( + SessionInfo.STAGED_SESSION_ACTIVATION_FAILED, errorMsg); } if (isApexSessionFailed(apexSessionInfo)) { String errorMsg = "APEX activation failed. Check logcat messages from apexd " @@ -640,10 +639,8 @@ public class StagingManager { errorMsg = "Session reverted due to crashing native process: " + mNativeFailureReason; } - session.setStagedSessionFailed(SessionInfo.STAGED_SESSION_ACTIVATION_FAILED, - errorMsg); - abortCheckpoint(session.sessionId, errorMsg); - return; + throw new PackageManagerException( + SessionInfo.STAGED_SESSION_ACTIVATION_FAILED, errorMsg); } if (!apexSessionInfo.isActivated && !apexSessionInfo.isSuccess) { // Apexd did not apply the session for some unknown reason. There is no @@ -651,42 +648,20 @@ public class StagingManager { // it as failed. final String errorMsg = "Staged session " + session.sessionId + "at boot " + "didn't activate nor fail. Marking it as failed anyway."; - session.setStagedSessionFailed(SessionInfo.STAGED_SESSION_ACTIVATION_FAILED, - errorMsg); - abortCheckpoint(session.sessionId, errorMsg); - return; + throw new PackageManagerException( + SessionInfo.STAGED_SESSION_ACTIVATION_FAILED, errorMsg); } } // Handle apk and apk-in-apex installation - try { - if (hasApex) { - checkInstallationOfApkInApexSuccessful(session); - snapshotAndRestoreForApexSession(session); - Slog.i(TAG, "APEX packages in session " + session.sessionId - + " were successfully activated. Proceeding with APK packages, if any"); - } - // The APEX part of the session is activated, proceed with the installation of APKs. - Slog.d(TAG, "Installing APK packages in session " + session.sessionId); - installApksInSession(session); - } catch (PackageManagerException e) { - session.setStagedSessionFailed(e.error, e.getMessage()); - abortCheckpoint(session.sessionId, e.getMessage()); - - // If checkpoint is not supported, we have to handle failure for one staged session. - if (!hasApex) { - return; - } - - if (!mApexManager.revertActiveSessions()) { - Slog.e(TAG, "Failed to abort APEXd session"); - } else { - Slog.e(TAG, - "Successfully aborted apexd session. Rebooting device in order to revert " - + "to the previous state of APEXd."); - mPowerManager.reboot(null); - } - return; + if (hasApex) { + checkInstallationOfApkInApexSuccessful(session); + snapshotAndRestoreForApexSession(session); + Slog.i(TAG, "APEX packages in session " + session.sessionId + + " were successfully activated. Proceeding with APK packages, if any"); } + // The APEX part of the session is activated, proceed with the installation of APKs. + Slog.d(TAG, "Installing APK packages in session " + session.sessionId); + installApksInSession(session); Slog.d(TAG, "Marking session " + session.sessionId + " as applied"); session.setStagedSessionApplied(); @@ -722,6 +697,25 @@ public class StagingManager { return ret; } + void onInstallationFailure(PackageInstallerSession session, PackageManagerException e) { + session.setStagedSessionFailed(e.error, e.getMessage()); + abortCheckpoint(session.sessionId, e.getMessage()); + + // If checkpoint is not supported, we have to handle failure for one staged session. + if (!sessionContainsApex(session)) { + return; + } + + if (!mApexManager.revertActiveSessions()) { + Slog.e(TAG, "Failed to abort APEXd session"); + } else { + Slog.e(TAG, + "Successfully aborted apexd session. Rebooting device in order to revert " + + "to the previous state of APEXd."); + mPowerManager.reboot(null); + } + } + @NonNull private PackageInstallerSession createAndWriteApkSession( @NonNull PackageInstallerSession originalSession, boolean preReboot) @@ -1185,7 +1179,16 @@ public class StagingManager { } else { // Session had already being marked ready. Start the checks to verify if there is any // follow-up work. - resumeSession(session); + try { + resumeSession(session); + } catch (PackageManagerException e) { + onInstallationFailure(session, e); + } catch (Exception e) { + Slog.e(TAG, "Staged install failed due to unhandled exception", e); + onInstallationFailure(session, new PackageManagerException( + SessionInfo.STAGED_SESSION_ACTIVATION_FAILED, + "Staged install failed due to unhandled exception: " + e)); + } } } From 4d60cca9f7061611e6b621202bac8d74b7c1c826 Mon Sep 17 00:00:00 2001 From: Mohammad Samiul Islam Date: Thu, 15 Oct 2020 01:05:13 +0100 Subject: [PATCH 3/3] [DO NOT MERGE] Prevent extra sessions owned by staged install from living across restarts If system server crashes due to external reasons (not due to exception in pre-reboot verifications), then the staged install will be retried. But, any extra sessions that have not been cleaned up will linger on until removed due to old age. And if the crash keeps on happening every time at the worst possible time, then we can have 1024 extra sessions created all with copies of apks in it. It will waste a lot of user's space. With this CL, we cleanup any leftover extra sessions whenever system server is restarted. Bug: 170784748 Test: manual Test: verified that without this CL if system server crashses after extra sessions are created but before they are cleaned up, then folders start to pile up in /data/app folder with copy of the apks inside. Change-Id: I5c19093853e28c668f882d37691959b1ac2464cc --- .../com/android/server/pm/PackageInstallerService.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index 330f995235071..56ef468407c4a 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -393,6 +393,8 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements Slog.w(TAG, "Abandoning old session created at " + session.createdMillis); valid = false; + } else if (isExtraSessionForStagedInstall(session)) { + valid = false; } else { valid = true; } @@ -423,6 +425,13 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements } } + // Extra sessions are created during staged install on temporary basis. They should not be + // allowed to live across system server restart. + private boolean isExtraSessionForStagedInstall(PackageInstallerSession session) { + return (session.params.installFlags & PackageManager.INSTALL_DRY_RUN) != 0 + || (session.params.installFlags & PackageManager.INSTALL_DISABLE_VERIFICATION) != 0; + } + @GuardedBy("mSessions") private void addHistoricalSessionLocked(PackageInstallerSession session) { CharArrayWriter writer = new CharArrayWriter();