From 00321b7026f029e84411970620633a0876aabdc9 Mon Sep 17 00:00:00 2001 From: Patrick Baumann Date: Tue, 9 Apr 2019 15:07:25 -0700 Subject: [PATCH] Adds sanity checks to session commit and abandon This change ensures we cannot commit or abandon a child session that has been added to a parent session. It also ensures that a child session being added to a parent has not already been added to another parent and that it has not yet been committed or destroyed. Fixes: 129534286 Test: atest CtsAtomicInstallTestCases:AtomicInstallTest Change-Id: Id1b3524acb5bf546f7239ae644fb3080c0d3128b --- .../android/content/pm/PackageInstaller.java | 5 ++-- .../server/pm/PackageInstallerSession.java | 30 ++++++++++++------- .../com/android/server/pm/StagingManager.java | 2 +- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/core/java/android/content/pm/PackageInstaller.java b/core/java/android/content/pm/PackageInstaller.java index 7fe840c11dfa4..37ba239ed3941 100644 --- a/core/java/android/content/pm/PackageInstaller.java +++ b/core/java/android/content/pm/PackageInstaller.java @@ -30,7 +30,6 @@ import android.annotation.UnsupportedAppUsage; import android.app.ActivityManager; import android.app.AppGlobals; import android.content.Intent; -import android.content.IntentFilter; import android.content.IntentSender; import android.content.pm.PackageManager.DeleteFlags; import android.content.pm.PackageManager.InstallReason; @@ -1221,7 +1220,7 @@ public class PackageInstaller { try { mSession.addChildSessionId(sessionId); } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); + e.rethrowFromSystemServer(); } } @@ -1235,7 +1234,7 @@ public class PackageInstaller { try { mSession.removeChildSessionId(sessionId); } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); + e.rethrowFromSystemServer(); } } } diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index a935c652be53e..74fb4b27100e2 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -841,6 +841,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @Override public void commit(@NonNull IntentSender statusReceiver, boolean forTransfer) { + if (hasParentSessionId()) { + throw new IllegalStateException( + "Session " + sessionId + " is a child of multi-package session " + + mParentSessionId + " and may not be committed directly."); + } if (!markAsCommitted(statusReceiver, forTransfer)) { return; } @@ -2037,6 +2042,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @Override public void abandon() { + if (hasParentSessionId()) { + throw new IllegalStateException( + "Session " + sessionId + " is a child of multi-package session " + + mParentSessionId + " and may not be abandoned directly."); + } synchronized (mLock) { assertCallerIsOwnerOrRootLocked(); @@ -2079,13 +2089,14 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } @Override - public void addChildSessionId(int childSessionId) throws RemoteException { + public void addChildSessionId(int childSessionId) { final PackageInstallerSession childSession = mSessionProvider.getSession(childSessionId); - if (childSession == null) { - throw new RemoteException("Unable to add child.", - new PackageManagerException("Child session " + childSessionId - + " does not exist"), - false, true).rethrowAsRuntimeException(); + if (childSession == null + || (childSession.hasParentSessionId() && childSession.mParentSessionId != sessionId) + || childSession.mCommitted + || childSession.mDestroyed) { + throw new IllegalStateException("Unable to add child session " + childSessionId + + " as it does not exist or is in an invalid state."); } synchronized (mLock) { assertCallerIsOwnerOrRootLocked(); @@ -2124,11 +2135,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { synchronized (mLock) { if (parentSessionId != SessionInfo.INVALID_ID && mParentSessionId != SessionInfo.INVALID_ID) { - throw new RemoteException("Unable to set parent session.", - new PackageManagerException( - "The parent of " + sessionId + " is" + " already set to " - + mParentSessionId), false, - true).rethrowAsRuntimeException(); + throw new IllegalStateException("The parent of " + sessionId + " is" + " already" + + "set to " + mParentSessionId); } this.mParentSessionId = parentSessionId; } diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index a0f0a3178d1b2..706480ed27162 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -487,7 +487,7 @@ public class StagingManager { } try { apkParentSession.addChildSessionId(apkChildSession.sessionId); - } catch (RemoteException e) { + } catch (IllegalStateException e) { Slog.e(TAG, "Failed to add a child session for installing the APK files", e); return false; }