diff --git a/core/java/android/content/pm/PackageInstaller.java b/core/java/android/content/pm/PackageInstaller.java index 1e6cea39b44dd..5d6867e143938 100644 --- a/core/java/android/content/pm/PackageInstaller.java +++ b/core/java/android/content/pm/PackageInstaller.java @@ -1211,6 +1211,9 @@ public class PackageInstaller { * Adds a session ID to the set of sessions that will be committed atomically * when this session is committed. * + *

If the parent is staged or has rollback enabled, all children must have + * the same properties. + * * @param sessionId the session ID to add to this multi-package session. */ public void addChildSessionId(int sessionId) { @@ -1480,6 +1483,9 @@ public class PackageInstaller { /** * Request that rollbacks be enabled or disabled for the given upgrade. * + *

If the parent session is staged or has rollback enabled, all children sessions + * must have the same properties. + * * @param enable set to {@code true} to enable, {@code false} to disable * @hide */ @@ -1607,6 +1613,9 @@ public class PackageInstaller { * multi-package. In that case, if any of the children sessions fail to install at reboot, * all the other children sessions are aborted as well. * + *

If the parent session is staged or has rollback enabled, all children sessions + * must have the same properties. + * * {@hide} */ @SystemApi @TestApi @@ -1626,6 +1635,11 @@ public class PackageInstaller { installFlags |= PackageManager.INSTALL_APEX; } + /** @hide */ + public boolean getEnableRollback() { + return (installFlags & PackageManager.INSTALL_ENABLE_ROLLBACK) != 0; + } + /** {@hide} */ public void dump(IndentingPrintWriter pw) { pw.printPair("mode", mode); diff --git a/core/java/android/content/pm/PackageManager.java b/core/java/android/content/pm/PackageManager.java index b190b34aa03fd..254238f68afe3 100644 --- a/core/java/android/content/pm/PackageManager.java +++ b/core/java/android/content/pm/PackageManager.java @@ -1397,6 +1397,14 @@ public abstract class PackageManager { */ public static final int INSTALL_FAILED_OTHER_STAGED_SESSION_IN_PROGRESS = -119; + /** + * Installation failed return code: one of the child sessions does not match the parent session + * in respect to staged or rollback enabled parameters. + * + * @hide + */ + public static final int INSTALL_FAILED_MULTIPACKAGE_INCONSISTENCY = -120; + /** @hide */ @IntDef(flag = true, prefix = { "DELETE_" }, value = { DELETE_KEEP_DATA, diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index ad17549d7448e..7c04cddcaad2c 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -396,6 +396,11 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements } finally { IoUtils.closeQuietly(fis); } + // After all of the sessions were loaded, they are ready to be sealed and validated + for (int i = 0; i < mSessions.size(); ++i) { + PackageInstallerSession session = mSessions.valueAt(i); + session.sealAndValidateIfNecessary(); + } } @GuardedBy("mSessions") diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index f1d4524cccac3..5d539a4b1dda7 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -231,6 +231,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @GuardedBy("mLock") private boolean mSealed = false; @GuardedBy("mLock") + private boolean mShouldBeSealed = false; + @GuardedBy("mLock") private boolean mCommitted = false; @GuardedBy("mLock") private boolean mRelinquished = false; @@ -430,6 +432,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { this.updatedMillis = createdMillis; this.stageDir = stageDir; this.stageCid = stageCid; + this.mShouldBeSealed = sealed; if (childSessionIds != null) { for (int childSessionId : childSessionIds) { mChildSessionIds.put(childSessionId, 0); @@ -450,16 +453,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { mStagedSessionErrorCode = stagedSessionErrorCode; mStagedSessionErrorMessage = stagedSessionErrorMessage != null ? stagedSessionErrorMessage : ""; - if (sealed) { - synchronized (mLock) { - try { - sealAndValidateLocked(); - } catch (PackageManagerException | IOException e) { - destroyInternal(); - throw new IllegalArgumentException(e); - } - } - } } public SessionInfo generateInfo() { @@ -932,6 +925,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @NonNull IntentSender statusReceiver, boolean forTransfer) { Preconditions.checkNotNull(statusReceiver); + List childSessions = getChildSessions(); + final boolean wasSealed; synchronized (mLock) { assertCallerIsOwnerOrRootLocked(); @@ -963,7 +958,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { wasSealed = mSealed; if (!mSealed) { try { - sealAndValidateLocked(); + sealAndValidateLocked(childSessions); } catch (IOException e) { throw new IllegalArgumentException(e); } catch (PackageManagerException e) { @@ -994,21 +989,91 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return true; } + /** Return a list of child sessions or null if the session is not multipackage + * + *

This method is handy to prevent potential deadlocks (b/123391593) + */ + private @Nullable List getChildSessions() { + List childSessions = null; + if (isMultiPackage()) { + final int[] childSessionIds = getChildSessionIds(); + childSessions = new ArrayList<>(childSessionIds.length); + for (int childSessionId : childSessionIds) { + childSessions.add(mSessionProvider.getSession(childSessionId)); + } + } + return childSessions; + } + + /** + * Assert multipackage install has consistent sessions. + * + * @throws PackageManagerException if child sessions don't match parent session + * in respect to staged and enable rollback parameters. + */ + @GuardedBy("mLock") + private void assertMultiPackageConsistencyLocked( + @NonNull List childSessions) throws PackageManagerException { + for (PackageInstallerSession childSession : childSessions) { + // It might be that the parent session is loaded before all of it's child sessions are, + // e.g. when reading sessions from XML. Those sessions will be null here, and their + // conformance with the multipackage params will be checked when they're loaded. + if (childSession == null) { + continue; + } + assertConsistencyWithLocked(childSession); + } + } + + /** + * Assert consistency with the given session. + * + * @throws PackageManagerException if other sessions doesn't match this session + * in respect to staged and enable rollback parameters. + */ + @GuardedBy("mLock") + private void assertConsistencyWithLocked(PackageInstallerSession other) + throws PackageManagerException { + // Session groups must be consistent wrt to isStaged parameter. Non-staging session + // cannot be grouped with staging sessions. + if (this.params.isStaged != other.params.isStaged) { + throw new PackageManagerException( + PackageManager.INSTALL_FAILED_MULTIPACKAGE_INCONSISTENCY, + "Multipackage Inconsistency: session " + other.sessionId + + " and session " + sessionId + + " have inconsistent staged settings"); + } + if (this.params.getEnableRollback() != other.params.getEnableRollback()) { + throw new PackageManagerException( + PackageManager.INSTALL_FAILED_MULTIPACKAGE_INCONSISTENCY, + "Multipackage Inconsistency: session " + other.sessionId + + " and session " + sessionId + + " have inconsistent rollback settings"); + } + } + /** * Seal the session to prevent further modification and validate the contents of it. * *

The session will be sealed after calling this method even if it failed. * + * @param childSessions the child sessions of a multipackage that will be checked for + * consistency. Can be null if session is not multipackage. * @throws PackageManagerException if the session was sealed but something went wrong. If the * session was sealed this is the only possible exception. */ @GuardedBy("mLock") - private void sealAndValidateLocked() throws PackageManagerException, IOException { + private void sealAndValidateLocked(List childSessions) + throws PackageManagerException, IOException { assertNoWriteFileTransfersOpenLocked(); assertPreparedAndNotDestroyedLocked("sealing of session"); mSealed = true; + if (childSessions != null) { + assertMultiPackageConsistencyLocked(childSessions); + } + if (params.isStaged) { final PackageInstallerSession activeSession = mStagingManager.getActiveSession(); final boolean anotherSessionAlreadyInProgress = @@ -1048,6 +1113,38 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } + /** + * If session should be sealed, then it's sealed to prevent further modification + * and then it's validated. + * + * If the session was sealed but something went wrong then it's destroyed. + * + *

This is meant to be called after all of the sessions are loaded and added to + * PackageInstallerService + */ + void sealAndValidateIfNecessary() { + synchronized (mLock) { + if (!mShouldBeSealed) { + return; + } + } + List childSessions = getChildSessions(); + synchronized (mLock) { + try { + sealAndValidateLocked(childSessions); + } catch (IOException e) { + throw new IllegalStateException(e); + } catch (PackageManagerException e) { + Slog.e(TAG, "Package not valid", e); + // Session is sealed but could not be verified, we need to destroy it. + destroyInternal(); + // Dispatch message to remove session from PackageInstallerService + dispatchSessionFinished( + e.error, ExceptionUtils.getCompleteMessage(e), null); + } + } + } + /** Update the timestamp of when the staged session last changed state */ public void markUpdated() { synchronized (mLock) { @@ -1076,12 +1173,14 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { throw new SecurityException("Can only transfer sessions that use public options"); } + List childSessions = getChildSessions(); + synchronized (mLock) { assertCallerIsOwnerOrRootLocked(); assertPreparedAndNotSealedLocked("transfer"); try { - sealAndValidateLocked(); + sealAndValidateLocked(childSessions); } catch (IOException e) { throw new IllegalStateException(e); } catch (PackageManagerException e) { @@ -1132,14 +1231,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { // outside of the lock, because reading the child // sessions with the lock held could lead to deadlock // (b/123391593). - List childSessions = null; - if (isMultiPackage()) { - final int[] childSessionIds = getChildSessionIds(); - childSessions = new ArrayList<>(childSessionIds.length); - for (int childSessionId : childSessionIds) { - childSessions.add(mSessionProvider.getSession(childSessionId)); - } - } + List childSessions = getChildSessions(); try { synchronized (mLock) { @@ -1965,15 +2057,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { + " does not exist"), false, true).rethrowAsRuntimeException(); } - // Session groups must be consistent wrt to isStaged parameter. Non-staging session - // cannot be grouped with staging sessions. - if (this.params.isStaged ^ childSession.params.isStaged) { - throw new RemoteException("Unable to add child.", - new PackageManagerException("Child session " + childSessionId - + " and parent session " + this.sessionId + " do not have consistent" - + " staging session settings."), - false, true); - } synchronized (mLock) { assertCallerIsOwnerOrRootLocked(); assertPreparedAndNotSealedLocked("addChildSessionId");