From 79825c6f2f8b46808e4b431fe52b3be78f1e8ac8 Mon Sep 17 00:00:00 2001 From: "T.J. Mercier" Date: Tue, 26 Jul 2022 16:36:07 +0000 Subject: [PATCH] Freeze package cgroup before killing Apps are able to prevent their death by forking multiple processes under different services and monitoring for the death of any of these. When a child death is detected, the remaining process restarts the terminating/terminated service before it's able to be killed itself. This is now prevented by freezing the entire cgroup of the package to be killed before killing the individual processes. After the kills are completed synchronously, the cgroup can be unfrozen to allow for restarts. Before freezing the cgroup, the binder interfaces of the processes about to be frozen are also frozen to prevent indefinite blocking by synchronous Binder callers. Bug: 236708592 Bug: 148425913 Test: raven:/ $ monkey -p com.haok.nirvana 1 Test: bash arg: -p Test: bash arg: com.haok.nirvana Test: bash arg: 1 Test: args: [-p, com.haok.nirvana, 1] Test: arg: "-p" Test: arg: "com.haok.nirvana" Test: arg: "1" Test: data="com.haok.nirvana" Test: Events injected: 1 Test: ## Network stats: elapsed time=14ms (0ms mobile, 0ms wifi, 14ms not connected) Test: raven:/ $ ps -eo pid,uid,ppid,name|grep 10266 Test: 2499 10266 823 com.haok.nirvana Test: 2577 10266 823 com.haok.nirvana:resident Test: 2584 10266 823 android.media Test: 2669 10266 1 app_d Test: 2672 10266 1 app_d Test: raven:/ $ am force-stop com.haok.nirvana Test: raven:/ $ ps -eo pid,uid,ppid,name|grep 10266 Test: 1|raven:/ $ Test: atest PermissionControllerHostTest Change-Id: I1718a9263a01d18da7e3d2e3771091cf8475e599 --- core/java/android/os/Process.java | 12 ++++ core/jni/android_util_Process.cpp | 15 ++++ .../android/server/am/CachedAppOptimizer.java | 12 ++-- .../com/android/server/am/ProcessList.java | 70 +++++++++++++++++-- .../com/android/server/am/ProcessRecord.java | 19 ++++- ...m_android_server_am_CachedAppOptimizer.cpp | 10 +-- .../android/server/am/BroadcastQueueTest.java | 2 +- 7 files changed, 123 insertions(+), 17 deletions(-) diff --git a/core/java/android/os/Process.java b/core/java/android/os/Process.java index c943a3d1861cc..e483328feb049 100644 --- a/core/java/android/os/Process.java +++ b/core/java/android/os/Process.java @@ -1481,6 +1481,18 @@ public class Process { */ public static final native int killProcessGroup(int uid, int pid); + /** + * Freeze the cgroup for the given UID. + * This cgroup may contain child cgroups which will also be frozen. If this cgroup or its + * children contain processes with Binder interfaces, those interfaces should be frozen before + * the cgroup to avoid blocking synchronous callers indefinitely. + * + * @param uid The UID to be frozen + * @param freeze true = freeze; false = unfreeze + * @hide + */ + public static final native void freezeCgroupUid(int uid, boolean freeze); + /** * Remove all process groups. Expected to be called when ActivityManager * is restarted. diff --git a/core/jni/android_util_Process.cpp b/core/jni/android_util_Process.cpp index b9d5ee4b30153..9501c8d49e53d 100644 --- a/core/jni/android_util_Process.cpp +++ b/core/jni/android_util_Process.cpp @@ -1252,6 +1252,20 @@ static jint android_os_Process_nativePidFdOpen(JNIEnv* env, jobject, jint pid, j return fd; } +void android_os_Process_freezeCgroupUID(JNIEnv* env, jobject clazz, jint uid, jboolean freeze) { + bool success = true; + + if (freeze) { + success = SetUserProfiles(uid, {"Frozen"}); + } else { + success = SetUserProfiles(uid, {"Unfrozen"}); + } + + if (!success) { + jniThrowRuntimeException(env, "Could not apply user profile"); + } +} + static const JNINativeMethod methods[] = { {"getUidForName", "(Ljava/lang/String;)I", (void*)android_os_Process_getUidForName}, {"getGidForName", "(Ljava/lang/String;)I", (void*)android_os_Process_getGidForName}, @@ -1293,6 +1307,7 @@ static const JNINativeMethod methods[] = { {"killProcessGroup", "(II)I", (void*)android_os_Process_killProcessGroup}, {"removeAllProcessGroups", "()V", (void*)android_os_Process_removeAllProcessGroups}, {"nativePidFdOpen", "(II)I", (void*)android_os_Process_nativePidFdOpen}, + {"freezeCgroupUid", "(IZ)V", (void*)android_os_Process_freezeCgroupUID}, }; int register_android_os_Process(JNIEnv* env) diff --git a/services/core/java/com/android/server/am/CachedAppOptimizer.java b/services/core/java/com/android/server/am/CachedAppOptimizer.java index 4574302485135..cbf0aae50e87a 100644 --- a/services/core/java/com/android/server/am/CachedAppOptimizer.java +++ b/services/core/java/com/android/server/am/CachedAppOptimizer.java @@ -112,6 +112,8 @@ public final class CachedAppOptimizer { private static final String ATRACE_COMPACTION_TRACK = "Compaction"; private static final String ATRACE_FREEZER_TRACK = "Freezer"; + private static final int FREEZE_BINDER_TIMEOUT_MS = 100; + // Defaults for phenotype flags. @VisibleForTesting static final Boolean DEFAULT_USE_COMPACTION = false; @VisibleForTesting static final Boolean DEFAULT_USE_FREEZER = true; @@ -929,11 +931,13 @@ public final class CachedAppOptimizer { * @param pid the target pid for which binder transactions are to be frozen * @param freeze specifies whether to flush transactions and then freeze (true) or unfreeze * binder for the specificed pid. + * @param timeoutMs the timeout in milliseconds to wait for the binder interface to freeze + * before giving up. * * @throws RuntimeException in case a flush/freeze operation could not complete successfully. * @return 0 if success, or -EAGAIN indicating there's pending transaction. */ - private static native int freezeBinder(int pid, boolean freeze); + public static native int freezeBinder(int pid, boolean freeze, int timeoutMs); /** * Retrieves binder freeze info about a process. @@ -1300,7 +1304,7 @@ public final class CachedAppOptimizer { long freezeTime = opt.getFreezeUnfreezeTime(); try { - freezeBinder(pid, false); + freezeBinder(pid, false, FREEZE_BINDER_TIMEOUT_MS); } catch (RuntimeException e) { Slog.e(TAG_AM, "Unable to unfreeze binder for " + pid + " " + app.processName + ". Killing it"); @@ -1355,7 +1359,7 @@ public final class CachedAppOptimizer { } Slog.d(TAG_AM, "quick sync unfreeze " + pid); try { - freezeBinder(pid, false); + freezeBinder(pid, false, FREEZE_BINDER_TIMEOUT_MS); } catch (RuntimeException e) { Slog.e(TAG_AM, "Unable to quick unfreeze binder for " + pid); return; @@ -1950,7 +1954,7 @@ public final class CachedAppOptimizer { // Freeze binder interface before the process, to flush any // transactions that might be pending. try { - if (freezeBinder(pid, true) != 0) { + if (freezeBinder(pid, true, FREEZE_BINDER_TIMEOUT_MS) != 0) { rescheduleFreeze(proc, "outstanding txns"); return; } diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java index d7b38482a4edc..42bfc4cc71306 100644 --- a/services/core/java/com/android/server/am/ProcessList.java +++ b/services/core/java/com/android/server/am/ProcessList.java @@ -32,6 +32,7 @@ import static android.os.Process.getFreeMemory; import static android.os.Process.getTotalMemory; import static android.os.Process.killProcessQuiet; import static android.os.Process.startWebView; +import static android.system.OsConstants.*; import static com.android.server.am.ActivityManagerDebugConfig.DEBUG_LRU; import static com.android.server.am.ActivityManagerDebugConfig.DEBUG_NETWORK; @@ -2711,6 +2712,50 @@ public final class ProcessList { } } + private static boolean freezePackageCgroup(int packageUID, boolean freeze) { + try { + Process.freezeCgroupUid(packageUID, freeze); + } catch (RuntimeException e) { + final String logtxt = freeze ? "freeze" : "unfreeze"; + Slog.e(TAG, "Unable to " + logtxt + " cgroup uid: " + packageUID + ": " + e); + return false; + } + return true; + } + + private static void freezeBinderAndPackageCgroup(ArrayList> procs, + int packageUID) { + // Freeze all binder processes under the target UID (whose cgroup is about to be frozen). + // Since we're going to kill these, we don't need to unfreze them later. + // The procs list may not include all processes under the UID cgroup, but unincluded + // processes (forks) should not be Binder users. + int N = procs.size(); + for (int i = 0; i < N; i++) { + final int uid = procs.get(i).first.uid; + final int pid = procs.get(i).first.getPid(); + int nRetries = 0; + // We only freeze the cgroup of the target package, so we do not need to freeze the + // Binder interfaces of dependant processes in other UIDs. + if (pid > 0 && uid == packageUID) { + try { + int rc; + do { + rc = CachedAppOptimizer.freezeBinder(pid, true, 10 /* timeout_ms */); + } while (rc == -EAGAIN && nRetries++ < 1); + if (rc != 0) Slog.e(TAG, "Unable to freeze binder for " + pid + ": " + rc); + } catch (RuntimeException e) { + Slog.e(TAG, "Unable to freeze binder for " + pid + ": " + e); + } + } + } + + // We freeze the entire UID (parent) cgroup so that newly-specialized processes also freeze + // despite being added to a new child cgroup. The cgroups of package dependant processes are + // not frozen, since it's possible this would freeze processes with no dependency on the + // package being killed here. + freezePackageCgroup(packageUID, true); + } + @GuardedBy({"mService", "mProcLock"}) boolean killPackageProcessesLSP(String packageName, int appId, int userId, int minOomAdj, boolean callerWillRestart, boolean allowRestart, @@ -2763,7 +2808,7 @@ public final class ProcessList { boolean shouldAllowRestart = false; // If no package is specified, we call all processes under the - // give user id. + // given user id. if (packageName == null) { if (userId != UserHandle.USER_ALL && app.userId != userId) { continue; @@ -2806,14 +2851,24 @@ public final class ProcessList { } } + final int packageUID = UserHandle.getUid(userId, appId); + final boolean doFreeze = appId >= Process.FIRST_APPLICATION_UID + && appId <= Process.LAST_APPLICATION_UID; + if (doFreeze) { + freezeBinderAndPackageCgroup(procs, packageUID); + } + int N = procs.size(); for (int i=0; i proc = procs.get(i); removeProcessLocked(proc.first, callerWillRestart, allowRestart || proc.second, - reasonCode, subReason, reason); + reasonCode, subReason, reason, !doFreeze /* async */); } killAppZygotesLocked(packageName, appId, userId, false /* force */); mService.updateOomAdjLocked(OomAdjuster.OOM_ADJ_REASON_PROCESS_END); + if (doFreeze) { + freezePackageCgroup(packageUID, false); + } return N > 0; } @@ -2821,12 +2876,19 @@ public final class ProcessList { boolean removeProcessLocked(ProcessRecord app, boolean callerWillRestart, boolean allowRestart, int reasonCode, String reason) { return removeProcessLocked(app, callerWillRestart, allowRestart, reasonCode, - ApplicationExitInfo.SUBREASON_UNKNOWN, reason); + ApplicationExitInfo.SUBREASON_UNKNOWN, reason, true); } @GuardedBy("mService") boolean removeProcessLocked(ProcessRecord app, boolean callerWillRestart, boolean allowRestart, int reasonCode, int subReason, String reason) { + return removeProcessLocked(app, callerWillRestart, allowRestart, reasonCode, subReason, + reason, true); + } + + @GuardedBy("mService") + boolean removeProcessLocked(ProcessRecord app, boolean callerWillRestart, + boolean allowRestart, int reasonCode, int subReason, String reason, boolean async) { final String name = app.processName; final int uid = app.uid; if (DEBUG_PROCESSES) Slog.d(TAG_PROCESSES, @@ -2863,7 +2925,7 @@ public final class ProcessList { needRestart = true; } } - app.killLocked(reason, reasonCode, subReason, true); + app.killLocked(reason, reasonCode, subReason, true, async); mService.handleAppDiedLocked(app, pid, willRestart, allowRestart, false /* fromBinderDied */); if (willRestart) { diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java index 482e6a792a90c..cc9071afa1c7d 100644 --- a/services/core/java/com/android/server/am/ProcessRecord.java +++ b/services/core/java/com/android/server/am/ProcessRecord.java @@ -1056,18 +1056,30 @@ class ProcessRecord implements WindowProcessListener { @GuardedBy("mService") void killLocked(String reason, @Reason int reasonCode, boolean noisy) { - killLocked(reason, reasonCode, ApplicationExitInfo.SUBREASON_UNKNOWN, noisy); + killLocked(reason, reasonCode, ApplicationExitInfo.SUBREASON_UNKNOWN, noisy, true); } @GuardedBy("mService") void killLocked(String reason, @Reason int reasonCode, @SubReason int subReason, boolean noisy) { - killLocked(reason, reason, reasonCode, subReason, noisy); + killLocked(reason, reason, reasonCode, subReason, noisy, true); } @GuardedBy("mService") void killLocked(String reason, String description, @Reason int reasonCode, @SubReason int subReason, boolean noisy) { + killLocked(reason, description, reasonCode, subReason, noisy, true); + } + + @GuardedBy("mService") + void killLocked(String reason, @Reason int reasonCode, @SubReason int subReason, + boolean noisy, boolean asyncKPG) { + killLocked(reason, reason, reasonCode, subReason, noisy, asyncKPG); + } + + @GuardedBy("mService") + void killLocked(String reason, String description, @Reason int reasonCode, + @SubReason int subReason, boolean noisy, boolean asyncKPG) { if (!mKilledByAm) { Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "kill"); if (reasonCode == ApplicationExitInfo.REASON_ANR @@ -1084,7 +1096,8 @@ class ProcessRecord implements WindowProcessListener { EventLog.writeEvent(EventLogTags.AM_KILL, userId, mPid, processName, mState.getSetAdj(), reason); Process.killProcessQuiet(mPid); - ProcessList.killProcessGroup(uid, mPid); + if (asyncKPG) ProcessList.killProcessGroup(uid, mPid); + else Process.killProcessGroup(uid, mPid); } else { mPendingStart = false; } diff --git a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp index 1d560780dc813..d3d69ae14e7e8 100644 --- a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp +++ b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp @@ -491,10 +491,10 @@ static void com_android_server_am_CachedAppOptimizer_compactProcess(JNIEnv*, job compactProcessOrFallback(pid, compactionFlags); } -static jint com_android_server_am_CachedAppOptimizer_freezeBinder( - JNIEnv *env, jobject clazz, jint pid, jboolean freeze) { - - jint retVal = IPCThreadState::freeze(pid, freeze, 100 /* timeout [ms] */); +static jint com_android_server_am_CachedAppOptimizer_freezeBinder(JNIEnv* env, jobject clazz, + jint pid, jboolean freeze, + jint timeout_ms) { + jint retVal = IPCThreadState::freeze(pid, freeze, timeout_ms); if (retVal != 0 && retVal != -EAGAIN) { jniThrowException(env, "java/lang/RuntimeException", "Unable to freeze/unfreeze binder"); } @@ -548,7 +548,7 @@ static const JNINativeMethod sMethods[] = { (void*)com_android_server_am_CachedAppOptimizer_getMemoryFreedCompaction}, {"compactSystem", "()V", (void*)com_android_server_am_CachedAppOptimizer_compactSystem}, {"compactProcess", "(II)V", (void*)com_android_server_am_CachedAppOptimizer_compactProcess}, - {"freezeBinder", "(IZ)I", (void*)com_android_server_am_CachedAppOptimizer_freezeBinder}, + {"freezeBinder", "(IZI)I", (void*)com_android_server_am_CachedAppOptimizer_freezeBinder}, {"getBinderFreezeInfo", "(I)I", (void*)com_android_server_am_CachedAppOptimizer_getBinderFreezeInfo}, {"getFreezerCheckPath", "()Ljava/lang/String;", diff --git a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java index 86d61698f6da0..f89d37807cdd4 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java @@ -364,7 +364,7 @@ public class BroadcastQueueTest { mActiveProcesses.remove(r); mRegisteredReceivers.remove(r.getPid()); return invocation.callRealMethod(); - }).when(r).killLocked(any(), any(), anyInt(), anyInt(), anyBoolean()); + }).when(r).killLocked(any(), any(), anyInt(), anyInt(), anyBoolean(), anyBoolean()); // If we're entirely dead, rely on default behaviors above if (dead) return r;