From 28c4e806ea6cb12b3b83af8447b6647471a15d38 Mon Sep 17 00:00:00 2001 From: Todd Kennedy Date: Wed, 13 Jul 2016 13:20:30 -0700 Subject: [PATCH] Remove subtle dependencies There was a very subtle dependency on the method used to allocate a session id and how that session id was stored. If the session id wasn't stored in the same synchronized block where the allocation method was called, it could have been possible to duplicate session ids. Instead of requiring callers of the allocation method to know that the value must be stored in a particular way, maintain a separate set of allocated session ids that is updated by the allocation method and prevents any potential race conditions. Change-Id: Ibd793b3851bf1a994e00d86f621180cac598b86e Fixes: 30089638 --- .../com/android/server/pm/PackageInstallerService.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index a3cf9c856ae5b..6a56fa6e629bd 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -182,6 +182,10 @@ public class PackageInstallerService extends IPackageInstaller.Stub { */ private final Random mRandom = new SecureRandom(); + /** All sessions allocated */ + @GuardedBy("mSessions") + private final SparseBooleanArray mAllocatedSessions = new SparseBooleanArray(); + @GuardedBy("mSessions") private final SparseArray mSessions = new SparseArray<>(); @@ -365,6 +369,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub { // keep details around for dumpsys. mHistoricalSessions.put(session.sessionId, session); } + mAllocatedSessions.put(session.sessionId, true); } } } @@ -768,8 +773,8 @@ public class PackageInstallerService extends IPackageInstaller.Stub { int sessionId; do { sessionId = mRandom.nextInt(Integer.MAX_VALUE - 1) + 1; - if (mSessions.get(sessionId) == null && mHistoricalSessions.get(sessionId) == null - && !mLegacySessions.get(sessionId, false)) { + if (!mAllocatedSessions.get(sessionId, false)) { + mAllocatedSessions.put(sessionId, true); return sessionId; } } while (n++ < 32);