From beec8e2b68aa285aabd6fc0e954e6f9916d67fb7 Mon Sep 17 00:00:00 2001 From: Todd Kennedy Date: Fri, 11 Aug 2017 10:15:04 -0700 Subject: [PATCH] Save observer in call to commit The observer used to be saved in a round-about way; being passed to the "commit" message handler and set there. The idea was that if there were multiple calls to commit() with different observers, it would be strange to wipe out the first observer during the second call to commit. However, setting the observer only in the commit handler means there are other methods [namely abandon() and setPermissionsResult()] where the observer will either not be called unless the commit message handler was run or the first observer might be wiped out with multiple commit calls. The only way to track this properly would be to generate a commit ID and assign an observer with that commit ID. However, there doesn't appear to be a legit use case for a caller to invoke commit multiple times on the same session with different observers. So, we just set the observer directly in the commit() method. Change-Id: I5e0d6c163ea84d1f49780aa899afb2e1cdf17584 Fixes: 64564511 Test: cts-tradefed run commandAndExit cts-dev -t android.appsecurity.cts.PkgInstallSignatureVerificationTest#testInstallEphemeralRequiresV2Signature -m CtsAppSecurityHostTestCases --- .../server/pm/PackageInstallerSession.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 082dd2badd1cd..0ecb4e1e80bd1 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -274,9 +274,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @Override public boolean handleMessage(Message msg) { synchronized (mLock) { - if (msg.obj != null) { - mRemoteObserver = (IPackageInstallObserver2) msg.obj; - } try { commitLocked(); } catch (PackageManagerException e) { @@ -666,7 +663,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } @Override - public void commit(IntentSender statusReceiver, boolean forTransfer) { + public void commit(@NonNull IntentSender statusReceiver, boolean forTransfer) { Preconditions.checkNotNull(statusReceiver); // Cache package manager data without the lock held @@ -679,6 +676,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { assertCallerIsOwnerOrRootLocked(); assertPreparedAndNotDestroyedLocked("commit"); + final PackageInstallObserverAdapter adapter = new PackageInstallObserverAdapter( + mContext, statusReceiver, sessionId, isInstallerDeviceOwnerLocked(), userId); + mRemoteObserver = adapter.getBinder(); + if (forTransfer) { mContext.enforceCallingOrSelfPermission(Manifest.permission.INSTALL_PACKAGES, null); @@ -712,9 +713,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { mActiveCount.incrementAndGet(); mCommitted = true; - final PackageInstallObserverAdapter adapter = new PackageInstallObserverAdapter( - mContext, statusReceiver, sessionId, isInstallerDeviceOwnerLocked(), userId); - mHandler.obtainMessage(MSG_COMMIT, adapter.getBinder()).sendToTarget(); + mHandler.obtainMessage(MSG_COMMIT).sendToTarget(); } if (!wasSealed) { @@ -1422,8 +1421,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } private void dispatchSessionFinished(int returnCode, String msg, Bundle extras) { - IPackageInstallObserver2 observer; - String packageName; + final IPackageInstallObserver2 observer; + final String packageName; synchronized (mLock) { mFinalStatus = returnCode; mFinalMessage = msg;