Merge changes from topic "abandon-race" into rvc-dev

* changes:
  Fix racing condition between session abandonment and its verification
  Persist destroyed staged sessions until they are cleaned up
This commit is contained in:
Mohammad Samiul Islam
2020-05-07 12:51:15 +00:00
committed by Android (Google) Code Review
4 changed files with 189 additions and 47 deletions

View File

@@ -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(

View File

@@ -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) {
@@ -1067,6 +1076,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}.
*/
@@ -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);
}
}

View File

@@ -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;
}
}
}
}
}

View File

@@ -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,