From 138c2dc68b7c46805ed92d0045e304bd252258ee Mon Sep 17 00:00:00 2001
From: Riddle Hsu <riddlehsu@google.com>
Date: Mon, 27 Dec 2021 17:49:18 +0800
Subject: [PATCH] Fix system crash by debug parameters of activity launch

Since AMS and ATMS use different lock, setDebugFlagsForStartingActivity
is posted to execute so it won't run on binder thread. And it may
throw security exception if not debuggable, that causes crash on system
server's thread (android.display).

Except dealing with the exception, it is more efficient to check before
calling setDebugFlagsForStartingActivity. And the check also avoids the
case of setDebugApp that enforces permission which is always passed on
non binder thread.

3 cases on a non-debuggable device (Build.IS_DEBUGGABLE is false):
 1. adb shell am start -n com.android.settings/.Settings -S -D
    > Debugger dialog should not popup.
 2. adb shell am start -n com.android.settings/.Settings -S \
      --start-profiler /data/local/tmp/p
    > System should not crash.
 3. Call IActivityTaskManager#startActivity by reflection to launch
    a non-debuggable app with debug flags.
    > System should not crash.

Bug: 208266893
Bug: 192247102
Test: atest AmProfileTests AmStartOptionsTests

Change-Id: I086635a50d8750580127f0b2868e5a241ac5f913
---
 .../server/wm/ActivityTaskSupervisor.java     | 33 +++++++++++++------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java
index 51c8dafa7c858..3cecce25d1952 100644
--- a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java
+++ b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java
@@ -641,22 +641,35 @@ public class ActivityTaskSupervisor implements RecentTasks.Callbacks {
             intent.setComponent(new ComponentName(
                     aInfo.applicationInfo.packageName, aInfo.name));
 
-            // Don't debug things in the system process
-            if (!aInfo.processName.equals("system")) {
-                if ((startFlags & (START_FLAG_DEBUG | START_FLAG_NATIVE_DEBUGGING
-                        | START_FLAG_TRACK_ALLOCATION)) != 0 || profilerInfo != null) {
-
+            final boolean requestDebug = (startFlags & (START_FLAG_DEBUG
+                    | START_FLAG_NATIVE_DEBUGGING | START_FLAG_TRACK_ALLOCATION)) != 0;
+            final boolean requestProfile = profilerInfo != null;
+            if (requestDebug || requestProfile) {
+                final boolean debuggable = (Build.IS_DEBUGGABLE
+                        || (aInfo.applicationInfo.flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0)
+                        && !aInfo.processName.equals("system");
+                if ((requestDebug && !debuggable) || (requestProfile
+                        && (!debuggable && !aInfo.applicationInfo.isProfileableByShell()))) {
+                    Slog.w(TAG, "Ignore debugging for non-debuggable app: " + aInfo.packageName);
+                } else {
                      // Mimic an AMS synchronous call by passing a message to AMS and wait for AMS
                      // to notify us that the task has completed.
                      // TODO(b/80414790) look into further untangling for the situation where the
                      // caller is on the same thread as the handler we are posting to.
                     synchronized (mService.mGlobalLock) {
                         // Post message to AMS.
-                        final Message msg = PooledLambda.obtainMessage(
-                                ActivityManagerInternal::setDebugFlagsForStartingActivity,
-                                mService.mAmInternal, aInfo, startFlags, profilerInfo,
-                                mService.mGlobalLock);
-                        mService.mH.sendMessage(msg);
+                        mService.mH.post(() -> {
+                            try {
+                                mService.mAmInternal.setDebugFlagsForStartingActivity(aInfo,
+                                        startFlags, profilerInfo, mService.mGlobalLock);
+                            } catch (Throwable e) {
+                                // Simply ignore it because the debugging doesn't take effect.
+                                Slog.w(TAG, e);
+                                synchronized (mService.mGlobalLockWithoutBoost) {
+                                    mService.mGlobalLockWithoutBoost.notifyAll();
+                                }
+                            }
+                        });
                         try {
                             mService.mGlobalLock.wait();
                         } catch (InterruptedException ignore) {