From 65067cb36da639e5a0cbd14bb14b9586dc721b63 Mon Sep 17 00:00:00 2001 From: Jackal Guo Date: Fri, 21 Jan 2022 15:51:00 +0800 Subject: [PATCH] Mitigate the races during installation When handling post-install during the installation process, some tasks are posted to PackageHandler and be executed after notifying the install observer (install initiator). The task includes force- stopping the package. If the install observer starts the app right after being notified, the ongoing force-stop will kill the process. The race happens. To mitigate the potential race, We should defer the notification until these tasks are done. Bug: 165012101 Test: atest -p services/core/java/com/android/server/pm Change-Id: Ia6b32f72f3d75b8a40c42e11d21f21c459db5299 Merged-In: Ia6b32f72f3d75b8a40c42e11d21f21c459db5299 (cherry picked from commit bad03aa3c9d2d1a5a875222936ccb9215d7ee137) --- .../content/pm/PackageManagerInternal.java | 5 ++++ .../server/am/ActivityManagerService.java | 2 ++ .../server/pm/InstallPackageHelper.java | 8 +++-- .../com/android/server/pm/PackageHandler.java | 9 ++++-- .../server/pm/PackageManagerService.java | 29 ++++++++++++++++--- 5 files changed, 44 insertions(+), 9 deletions(-) diff --git a/services/core/java/android/content/pm/PackageManagerInternal.java b/services/core/java/android/content/pm/PackageManagerInternal.java index 111bd340a3d1c..e6953f0032c76 100644 --- a/services/core/java/android/content/pm/PackageManagerInternal.java +++ b/services/core/java/android/content/pm/PackageManagerInternal.java @@ -656,6 +656,11 @@ public abstract class PackageManagerInternal { */ public abstract void notifyPackageUse(String packageName, int reason); + /** + * Notify the package is force stopped. + */ + public abstract void onPackageProcessKilledForUninstall(String packageName); + /** * Returns a package object for the given package name. */ diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 6813f3f83135d..34a986ae32c6a 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -13596,6 +13596,8 @@ public class ActivityManagerService extends IActivityManager.Stub intent.getIntExtra(Intent.EXTRA_UID, -1)), false, true, true, false, fullUninstall, userId, removed ? "pkg removed" : "pkg changed"); + getPackageManagerInternal() + .onPackageProcessKilledForUninstall(ssp); } else { // Kill any app zygotes always, since they can't fork new // processes with references to the old code diff --git a/services/core/java/com/android/server/pm/InstallPackageHelper.java b/services/core/java/com/android/server/pm/InstallPackageHelper.java index c09c904ff9317..c89b194a5cfe1 100644 --- a/services/core/java/com/android/server/pm/InstallPackageHelper.java +++ b/services/core/java/com/android/server/pm/InstallPackageHelper.java @@ -2889,9 +2889,13 @@ final class InstallPackageHelper { } } - final boolean deferInstallObserver = succeeded && update && !killApp; + final boolean deferInstallObserver = succeeded && update; if (deferInstallObserver) { - mPm.scheduleDeferredNoKillInstallObserver(res, installObserver); + if (killApp) { + mPm.scheduleDeferredPendingKillInstallObserver(res, installObserver); + } else { + mPm.scheduleDeferredNoKillInstallObserver(res, installObserver); + } } else { mPm.notifyInstallObserver(res, installObserver); } diff --git a/services/core/java/com/android/server/pm/PackageHandler.java b/services/core/java/com/android/server/pm/PackageHandler.java index b028a2cef2a56..e8faca9765f85 100644 --- a/services/core/java/com/android/server/pm/PackageHandler.java +++ b/services/core/java/com/android/server/pm/PackageHandler.java @@ -24,6 +24,7 @@ import static com.android.server.pm.PackageManagerService.DEBUG_INSTALL; import static com.android.server.pm.PackageManagerService.DEFAULT_UNUSED_STATIC_SHARED_LIB_MIN_CACHE_PERIOD; import static com.android.server.pm.PackageManagerService.DEFERRED_NO_KILL_INSTALL_OBSERVER; import static com.android.server.pm.PackageManagerService.DEFERRED_NO_KILL_POST_DELETE; +import static com.android.server.pm.PackageManagerService.DEFERRED_PENDING_KILL_INSTALL_OBSERVER; import static com.android.server.pm.PackageManagerService.DOMAIN_VERIFICATION; import static com.android.server.pm.PackageManagerService.ENABLE_ROLLBACK_STATUS; import static com.android.server.pm.PackageManagerService.ENABLE_ROLLBACK_TIMEOUT; @@ -126,10 +127,12 @@ final class PackageHandler extends Handler { } } } break; - case DEFERRED_NO_KILL_INSTALL_OBSERVER: { - String packageName = (String) msg.obj; + case DEFERRED_NO_KILL_INSTALL_OBSERVER: + case DEFERRED_PENDING_KILL_INSTALL_OBSERVER: { + final String packageName = (String) msg.obj; if (packageName != null) { - mPm.notifyInstallObserver(packageName); + final boolean killApp = msg.what == DEFERRED_PENDING_KILL_INSTALL_OBSERVER; + mPm.notifyInstallObserver(packageName, killApp); } } break; case WRITE_SETTINGS: { diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index c05faf14cae40..429123964e8d7 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -840,6 +840,9 @@ public class PackageManagerService extends IPackageManager.Stub private final Map> mNoKillInstallObservers = Collections.synchronizedMap(new HashMap<>()); + private final Map> + mPendingKillInstallObservers = Collections.synchronizedMap(new HashMap<>()); + // Internal interface for permission manager final PermissionManagerServiceInternal mPermissionManager; @@ -887,9 +890,11 @@ public class PackageManagerService extends IPackageManager.Stub static final int CHECK_PENDING_INTEGRITY_VERIFICATION = 26; static final int DOMAIN_VERIFICATION = 27; static final int PRUNE_UNUSED_STATIC_SHARED_LIBRARIES = 28; + static final int DEFERRED_PENDING_KILL_INSTALL_OBSERVER = 29; static final int DEFERRED_NO_KILL_POST_DELETE_DELAY_MS = 3 * 1000; private static final int DEFERRED_NO_KILL_INSTALL_OBSERVER_DELAY_MS = 500; + private static final int DEFERRED_PENDING_KILL_INSTALL_OBSERVER_DELAY_MS = 1000; static final int WRITE_SETTINGS_DELAY = 10*1000; // 10 seconds @@ -1166,13 +1171,14 @@ public class PackageManagerService extends IPackageManager.Stub Computer computer = snapshotComputer(); ArraySet packagesToNotify = computer.getNotifyPackagesForReplacedReceived(packages); for (int index = 0; index < packagesToNotify.size(); index++) { - notifyInstallObserver(packagesToNotify.valueAt(index)); + notifyInstallObserver(packagesToNotify.valueAt(index), false /* killApp */); } } - void notifyInstallObserver(String packageName) { - Pair pair = - mNoKillInstallObservers.remove(packageName); + void notifyInstallObserver(String packageName, boolean killApp) { + final Pair pair = + killApp ? mPendingKillInstallObservers.remove(packageName) + : mNoKillInstallObservers.remove(packageName); if (pair != null) { notifyInstallObserver(pair.first, pair.second); @@ -1211,6 +1217,15 @@ public class PackageManagerService extends IPackageManager.Stub delay ? getPruneUnusedSharedLibrariesDelay() : 0); } + void scheduleDeferredPendingKillInstallObserver(PackageInstalledInfo info, + IPackageInstallObserver2 observer) { + final String packageName = info.mPkg.getPackageName(); + mPendingKillInstallObservers.put(packageName, Pair.create(info, observer)); + final Message message = mHandler.obtainMessage(DEFERRED_PENDING_KILL_INSTALL_OBSERVER, + packageName); + mHandler.sendMessageDelayed(message, DEFERRED_PENDING_KILL_INSTALL_OBSERVER_DELAY_MS); + } + private static long getPruneUnusedSharedLibrariesDelay() { return SystemProperties.getLong("debug.pm.prune_unused_shared_libraries_delay", PRUNE_UNUSED_SHARED_LIBRARIES_DELAY); @@ -7392,6 +7407,12 @@ public class PackageManagerService extends IPackageManager.Stub } } + @Override + public void onPackageProcessKilledForUninstall(String packageName) { + mHandler.post(() -> PackageManagerService.this.notifyInstallObserver(packageName, + true /* killApp */)); + } + @Override public SparseArray getAppsWithSharedUserIds() { return mComputer.getAppsWithSharedUserIds();