diff options
4 files changed, 189 insertions, 47 deletions
diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index 3367cd556b2b..5b5f334803e5 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -676,7 +676,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements session = new PackageInstallerSession(mInternalCallback, mContext, mPm, this, mInstallThread.getLooper(), mStagingManager, sessionId, userId, callingUid, installSource, params, createdMillis, - stageDir, stageCid, null, false, false, false, null, SessionInfo.INVALID_ID, + stageDir, stageCid, null, false, false, false, false, null, SessionInfo.INVALID_ID, false, false, false, SessionInfo.STAGED_SESSION_NO_ERROR, ""); synchronized (mSessions) { @@ -830,7 +830,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements synchronized (mSessions) { final PackageInstallerSession session = mSessions.get(sessionId); - return session != null + return (session != null && !(session.isStaged() && session.isDestroyed())) ? session.generateInfoForCaller(true /*withIcon*/, Binder.getCallingUid()) : null; } @@ -851,7 +851,8 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements synchronized (mSessions) { for (int i = 0; i < mSessions.size(); i++) { final PackageInstallerSession session = mSessions.valueAt(i); - if (session.userId == userId && !session.hasParentSessionId()) { + if (session.userId == userId && !session.hasParentSessionId() + && !(session.isStaged() && session.isDestroyed())) { result.add(session.generateInfoForCaller(false, callingUid)); } } @@ -1269,7 +1270,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements public void onStagedSessionChanged(PackageInstallerSession session) { session.markUpdated(); writeSessionsAsync(); - if (mOkToSendBroadcasts) { + if (mOkToSendBroadcasts && !session.isDestroyed()) { // we don't scrub the data here as this is sent only to the installer several // privileged system packages mPm.sendSessionUpdatedBroadcast( diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index d3f377e135cb..e0d057a8d7f3 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -190,6 +190,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { private static final String ATTR_SESSION_STAGE_CID = "sessionStageCid"; private static final String ATTR_PREPARED = "prepared"; private static final String ATTR_COMMITTED = "committed"; + private static final String ATTR_DESTROYED = "destroyed"; private static final String ATTR_SEALED = "sealed"; private static final String ATTR_MULTI_PACKAGE = "multiPackage"; private static final String ATTR_PARENT_SESSION_ID = "parentSessionId"; @@ -533,7 +534,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { int sessionId, int userId, int installerUid, @NonNull InstallSource installSource, SessionParams params, long createdMillis, File stageDir, String stageCid, InstallationFile[] files, boolean prepared, - boolean committed, boolean sealed, + boolean committed, boolean destroyed, boolean sealed, @Nullable int[] childSessionIds, int parentSessionId, boolean isReady, boolean isFailed, boolean isApplied, int stagedSessionErrorCode, String stagedSessionErrorMessage) { @@ -579,6 +580,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { mPrepared = prepared; mCommitted = committed; + mDestroyed = destroyed; mStagedSessionReady = isReady; mStagedSessionFailed = isFailed; mStagedSessionApplied = isApplied; @@ -713,6 +715,13 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } + /** {@hide} */ + boolean isDestroyed() { + synchronized (mLock) { + return mDestroyed; + } + } + /** Returns true if a staged session has reached a final state and can be forgotten about */ public boolean isStagedAndInTerminalState() { synchronized (mLock) { @@ -1068,6 +1077,19 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } /** + * Check if the caller is the owner of this session. Otherwise throw a + * {@link SecurityException}. + */ + @GuardedBy("mLock") + private void assertCallerIsOwnerOrRootOrSystemLocked() { + final int callingUid = Binder.getCallingUid(); + if (callingUid != Process.ROOT_UID && callingUid != mInstallerUid + && callingUid != Process.SYSTEM_UID) { + throw new SecurityException("Session does not belong to uid " + callingUid); + } + } + + /** * If anybody is reading or writing data of the session, throw an {@link SecurityException}. */ @GuardedBy("mLock") @@ -2552,7 +2574,13 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { + mParentSessionId + " and may not be abandoned directly."); } synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); + if (params.isStaged && 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(); + } if (isStagedAndInTerminalState()) { // We keep the session in the database if it's in a finalized state. It will be @@ -2562,11 +2590,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return; } if (mCommitted && params.isStaged) { - synchronized (mLock) { - mDestroyed = true; + mDestroyed = true; + if (!mStagingManager.abortCommittedSessionLocked(this)) { + // Do not clean up the staged session from system. It is not safe yet. + mCallback.onStagedSessionChanged(this); + return; } - mStagingManager.abortCommittedSession(this); - cleanStageDir(); } @@ -2926,6 +2955,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { /** {@hide} */ void setStagedSessionReady() { synchronized (mLock) { + if (mDestroyed) return; // Do not allow destroyed staged session to change state mStagedSessionReady = true; mStagedSessionApplied = false; mStagedSessionFailed = false; @@ -2939,6 +2969,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { void setStagedSessionFailed(@StagedSessionErrorCode int errorCode, String errorMessage) { synchronized (mLock) { + if (mDestroyed) return; // Do not allow destroyed staged session to change state mStagedSessionReady = false; mStagedSessionApplied = false; mStagedSessionFailed = true; @@ -2953,6 +2984,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { /** {@hide} */ void setStagedSessionApplied() { synchronized (mLock) { + if (mDestroyed) return; // Do not allow destroyed staged session to change state mStagedSessionReady = false; mStagedSessionApplied = true; mStagedSessionFailed = false; @@ -3197,7 +3229,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { */ void write(@NonNull XmlSerializer out, @NonNull File sessionsDir) throws IOException { synchronized (mLock) { - if (mDestroyed) { + if (mDestroyed && !params.isStaged) { return; } @@ -3223,6 +3255,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } writeBooleanAttribute(out, ATTR_PREPARED, isPrepared()); writeBooleanAttribute(out, ATTR_COMMITTED, isCommitted()); + writeBooleanAttribute(out, ATTR_DESTROYED, isDestroyed()); writeBooleanAttribute(out, ATTR_SEALED, isSealed()); writeBooleanAttribute(out, ATTR_MULTI_PACKAGE, params.isMultiPackage); @@ -3352,6 +3385,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { final String stageCid = readStringAttribute(in, ATTR_SESSION_STAGE_CID); final boolean prepared = readBooleanAttribute(in, ATTR_PREPARED, true); final boolean committed = readBooleanAttribute(in, ATTR_COMMITTED); + final boolean destroyed = readBooleanAttribute(in, ATTR_DESTROYED); final boolean sealed = readBooleanAttribute(in, ATTR_SEALED); final int parentSessionId = readIntAttribute(in, ATTR_PARENT_SESSION_ID, SessionInfo.INVALID_ID); @@ -3473,7 +3507,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return new PackageInstallerSession(callback, context, pm, sessionProvider, installerThread, stagingManager, sessionId, userId, installerUid, installSource, params, createdMillis, stageDir, stageCid, fileArray, - prepared, committed, sealed, childSessionIdsArray, parentSessionId, + prepared, committed, destroyed, sealed, childSessionIdsArray, parentSessionId, isReady, isFailed, isApplied, stagedSessionErrorCode, stagedSessionErrorMessage); } } diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 9a297d601a6b..a83fa32ec9a9 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -61,6 +61,7 @@ import android.text.TextUtils; 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; @@ -137,6 +138,9 @@ public class StagingManager { synchronized (mStagedSessions) { for (int i = 0; i < mStagedSessions.size(); i++) { final PackageInstallerSession stagedSession = mStagedSessions.valueAt(i); + if (stagedSession.isDestroyed()) { + continue; + } result.add(stagedSession.generateInfoForCaller(false /*icon*/, callingUid)); } } @@ -202,7 +206,7 @@ public class StagingManager { final IntArray childSessionIds = new IntArray(); if (session.isMultiPackage()) { for (int id : session.getChildSessionIds()) { - if (isApexSession(mStagedSessions.get(id))) { + if (isApexSession(getStagedSession(id))) { childSessionIds.add(id); } } @@ -797,6 +801,8 @@ public class StagingManager { + session.sessionId + " [" + errorMessage + "]"); session.setStagedSessionFailed( SessionInfo.STAGED_SESSION_VERIFICATION_FAILED, errorMessage); + mPreRebootVerificationHandler.onPreRebootVerificationComplete( + session.sessionId); return; } mPreRebootVerificationHandler.notifyPreRebootVerification_Apk_Complete( @@ -880,7 +886,8 @@ public class StagingManager { synchronized (mStagedSessions) { for (int i = 0; i < mStagedSessions.size(); i++) { final PackageInstallerSession stagedSession = mStagedSessions.valueAt(i); - if (!stagedSession.isCommitted() || stagedSession.isStagedAndInTerminalState()) { + if (!stagedSession.isCommitted() || stagedSession.isStagedAndInTerminalState() + || stagedSession.isDestroyed()) { continue; } if (stagedSession.isMultiPackage()) { @@ -943,27 +950,68 @@ public class StagingManager { } } - void abortCommittedSession(@NonNull PackageInstallerSession session) { + /** + * <p>Abort committed staged session + * + * <p>This method must be called while holding {@link PackageInstallerSession.mLock}. + * + * <p>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}. + * + * <p> 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) { + int sessionId = session.sessionId; if (session.isStagedSessionApplied()) { - Slog.w(TAG, "Cannot abort applied session : " + session.sessionId); - return; + Slog.w(TAG, "Cannot abort applied session : " + sessionId); + return false; + } + if (!session.isDestroyed()) { + throw new IllegalStateException("Committed session must be destroyed before aborting it" + + " from StagingManager"); + } + if (getStagedSession(sessionId) == null) { + Slog.w(TAG, "Session " + sessionId + " has been abandoned already"); + return false; } - abortSession(session); - boolean hasApex = sessionContainsApex(session); - if (hasApex) { - ApexSessionInfo apexSession = mApexManager.getStagedSessionInfo(session.sessionId); - if (apexSession == null || isApexSessionFinalized(apexSession)) { - Slog.w(TAG, - "Cannot abort session " + session.sessionId - + " because it is not active or APEXD is not reachable"); - return; - } - try { - mApexManager.abortStagedSession(session.sessionId); - } catch (Exception ignore) { + // 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; + } + + // A session could be marked ready once its pre-reboot verification ends + if (session.isStagedSessionReady()) { + if (sessionContainsApex(session)) { + try { + ApexSessionInfo apexSession = + mApexManager.getStagedSessionInfo(session.sessionId); + if (apexSession == null || isApexSessionFinalized(apexSession)) { + Slog.w(TAG, + "Cannot abort session " + session.sessionId + + " because it is not active."); + } else { + mApexManager.abortStagedSession(session.sessionId); + } + } catch (Exception e) { + // Failed to contact apexd service. The apex might still be staged. We can still + // safely cleanup the staged session since pre-reboot verification is complete. + // Also, cleaning up the stageDir prevents the apex from being activated. + Slog.w(TAG, "Could not contact apexd to abort staged session " + sessionId); + } } } + + // 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; } private boolean isApexSessionFinalized(ApexSessionInfo session) { @@ -1042,6 +1090,11 @@ public class StagingManager { // Final states, nothing to do. return; } + if (session.isDestroyed()) { + // Device rebooted before abandoned session was cleaned up. + session.abandon(); + return; + } if (!session.isStagedSessionReady()) { // The framework got restarted before the pre-reboot verification could complete, // restart the verification. @@ -1124,10 +1177,20 @@ public class StagingManager { } } + private PackageInstallerSession getStagedSession(int sessionId) { + PackageInstallerSession session; + synchronized (mStagedSessions) { + session = mStagedSessions.get(sessionId); + } + return session; + } + private final class PreRebootVerificationHandler extends Handler { // 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); @@ -1155,13 +1218,15 @@ public class StagingManager { @Override public void handleMessage(Message msg) { final int sessionId = msg.arg1; - final PackageInstallerSession session; - synchronized (mStagedSessions) { - session = mStagedSessions.get(sessionId); - } - // Maybe session was aborted before pre-reboot verification was complete + final PackageInstallerSession session = getStagedSession(sessionId); if (session == null) { - Slog.d(TAG, "Stopping pre-reboot verification for sessionId: " + sessionId); + Slog.wtf(TAG, "Session disappeared in the middle of pre-reboot verification: " + + sessionId); + return; + } + if (session.isDestroyed()) { + // No point in running verification on a destroyed session + onPreRebootVerificationComplete(sessionId); return; } switch (msg.what) { @@ -1200,9 +1265,40 @@ public class StagingManager { mPendingSessionIds.add(sessionId); return; } + + PackageInstallerSession session = getStagedSession(sessionId); + synchronized (mVerificationRunning) { + // Do not start verification on a session that has been abandoned + if (session == null || session.isDestroyed()) { + return; + } + Slog.d(TAG, "Starting preRebootVerification for session " + sessionId); + mVerificationRunning.put(sessionId, true); + } obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, 0).sendToTarget(); } + // 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 notifyPreRebootVerification_Start_Complete(int sessionId) { obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APEX, sessionId, 0).sendToTarget(); } @@ -1221,8 +1317,6 @@ public class StagingManager { * See {@link PreRebootVerificationHandler} to see all nodes of pre reboot verification */ private void handlePreRebootVerification_Start(@NonNull PackageInstallerSession session) { - Slog.d(TAG, "Starting preRebootVerification for session " + session.sessionId); - if ((session.params.installFlags & PackageManager.INSTALL_ENABLE_ROLLBACK) != 0) { // If rollback is enabled for this session, we call through to the RollbackManager // with the list of sessions it must enable rollback for. Note that @@ -1269,6 +1363,7 @@ public class StagingManager { } } catch (PackageManagerException e) { session.setStagedSessionFailed(e.error, e.getMessage()); + onPreRebootVerificationComplete(session.sessionId); return; } @@ -1301,6 +1396,7 @@ public class StagingManager { // TODO(b/118865310): abort the session on apexd. } catch (PackageManagerException e) { session.setStagedSessionFailed(e.error, e.getMessage()); + onPreRebootVerificationComplete(session.sessionId); } } @@ -1323,9 +1419,18 @@ public class StagingManager { Slog.e(TAG, "Failed to get hold of StorageManager", e); session.setStagedSessionFailed(SessionInfo.STAGED_SESSION_UNKNOWN, "Failed to get hold of StorageManager"); + onPreRebootVerificationComplete(session.sessionId); return; } + // Stop pre-reboot verification before marking session ready. From this point on, if we + // abandon the session then it will be cleaned up immediately. If session is abandoned + // after this point, then even if for some reason system tries to install the session + // 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); + // 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 // in the inconsistent state: @@ -1337,15 +1442,16 @@ public class StagingManager { // only apex part of the train will be applied, leaving device in an inconsistent state. Slog.d(TAG, "Marking session " + session.sessionId + " as ready"); session.setStagedSessionReady(); - final boolean hasApex = sessionContainsApex(session); - if (!hasApex) { - // Session doesn't contain apex, nothing to do. - return; - } - try { - mApexManager.markStagedSessionReady(session.sessionId); - } catch (PackageManagerException e) { - session.setStagedSessionFailed(e.error, e.getMessage()); + if (session.isStagedSessionReady()) { + final boolean hasApex = sessionContainsApex(session); + if (hasApex) { + try { + mApexManager.markStagedSessionReady(session.sessionId); + } catch (PackageManagerException e) { + session.setStagedSessionFailed(e.error, e.getMessage()); + return; + } + } } } } diff --git a/services/tests/servicestests/src/com/android/server/pm/PackageInstallerSessionTest.java b/services/tests/servicestests/src/com/android/server/pm/PackageInstallerSessionTest.java index d4edab44bae3..63d797e9b95c 100644 --- a/services/tests/servicestests/src/com/android/server/pm/PackageInstallerSessionTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/PackageInstallerSessionTest.java @@ -178,6 +178,7 @@ public class PackageInstallerSessionTest { /* files */ null, /* prepared */ true, /* committed */ true, + /* destroyed */ staged ? true : false, /* sealed */ false, // Setting to true would trigger some PM logic. /* childSessionIds */ childSessionIds != null ? childSessionIds : new int[0], /* parentSessionId */ parentSessionId, |