From 3aa1f5aa168d1e14d8a0a4c54b44aecbfe26cbd2 Mon Sep 17 00:00:00 2001 From: Mohammad Samiul Islam Date: Mon, 29 Jun 2020 12:31:51 +0100 Subject: [PATCH] Fix deadlock in StagingManager.getSessions() method When we stage a session we acquire the PackageInstallerSession#mLock first and then StagingManager#mStagedSessions lock. But in getSessions() method, we acquire the locks in reverse order which can lead to deadlock: we first acquire StagingManager#mStagedSessions in getSessions() and then while checking if session is destroyed, we acquire PackageInstallerSession#mLock in isDestroyed() method. By moving the staged session retrieval logic to PackageInstallerService, we avoid acquiring the StagingManager#mStagedSession lock. This avoids deadlock. Bug: 160113544 Test: atest StagedInstallTest Change-Id: I7a1abf44db27d4673d43e9b0886610a5bd8b41f6 Merged-In: I7a1abf44db27d4673d43e9b0886610a5bd8b41f6 (cherry picked from commit c45b6a146c796ea3a006733d0055b48dc1961edd) --- .../server/pm/PackageInstallerService.java | 11 ++++++++++- .../com/android/server/pm/StagingManager.java | 15 --------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index 91b2ea1853fa8..2f149266e289a 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -865,7 +865,16 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements @Override public ParceledListSlice getStagedSessions() { - return mStagingManager.getSessions(Binder.getCallingUid()); + final List result = new ArrayList<>(); + synchronized (mSessions) { + for (int i = 0; i < mSessions.size(); i++) { + final PackageInstallerSession session = mSessions.valueAt(i); + if (session.isStaged() && !session.isDestroyed()) { + result.add(session.generateInfoForCaller(false, Binder.getCallingUid())); + } + } + } + return new ParceledListSlice<>(result); } @Override diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 6cd66c642a064..5b1c0fdb8249e 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -38,7 +38,6 @@ import android.content.pm.PackageManagerInternal; import android.content.pm.PackageParser.PackageParserException; import android.content.pm.PackageParser.SigningDetails; import android.content.pm.PackageParser.SigningDetails.SignatureSchemeVersion; -import android.content.pm.ParceledListSlice; import android.content.pm.parsing.PackageInfoWithoutStateUtils; import android.content.rollback.IRollbackManager; import android.content.rollback.RollbackInfo; @@ -180,20 +179,6 @@ public class StagingManager { } } - ParceledListSlice getSessions(int callingUid) { - final List result = new ArrayList<>(); - 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)); - } - } - return new ParceledListSlice<>(result); - } - /** * Validates the signature used to sign the container of the new apex package *