DO NOT MERGE - Kill apps outright for API contract violations

...rather than relying on in-app code to perform the shutdown.

Backport of security fix.

Bug: 128649910
Bug: 140108616
Test: manual
Test: atest OsHostTests#testForegroundServiceBadNotification

[basilgello: back-port to 14.1:
- core/java/android/app/IActivityManager.aidl -> core/java/android/app/IActivityManager.java,
- no serviceForegroundCrash in services/core/java/com/android/server/am/ActiveServices.java,
- no runCrash in services/core/java/com/android/server/am/ActivityManagerShellCommand.java,
- add argument to ActivityManagerProxy,
- no mNotificationLock and ForegroundService,
- adjust args count (remove '-1') in killMisbehavingService]
Signed-off-by: Vasyl Gello <vasek.gello@gmail.com>
Change-Id: I94d9de50bb03c33666471e3dbd9c721e9278f7cb
Merged-In: I94d9de50bb03c33666471e3dbd9c721e9278f7cb
(cherry picked from commit a79b6ba5c59dc6aaa8adbe1ffa3ee4b761f45e7f)

diff --git a/core/java/android/app/ActivityManagerNative.java b/core/java/android/app/ActivityManagerNative.java
index 50479c8..8effc93 100644 (file)
--- a/core/java/android/app/ActivityManagerNative.java
+++ b/core/java/android/app/ActivityManagerNative.java
@@ -1959,7 +1959,8 @@ public abstract class ActivityManagerNative extends Binder implements IActivityM
             int initialPid = data.readInt();
             String packageName = data.readString();
             String message = data.readString();
-            crashApplication(uid, initialPid, packageName, message);
+            crashApplication(uid, initialPid, packageName, message,
+                    false /*force*/);
             reply.writeNoException();
             return true;
         }
@@ -5523,7 +5524,7 @@ class ActivityManagerProxy implements IActivityManager
     }
 
     public void crashApplication(int uid, int initialPid, String packageName,
-            String message) throws RemoteException {
+            String message, boolean force) throws RemoteException {
         Parcel data = Parcel.obtain();
         Parcel reply = Parcel.obtain();
         data.writeInterfaceToken(IActivityManager.descriptor);

diff --git a/core/java/android/app/IActivityManager.java b/core/java/android/app/IActivityManager.java
index 5a4470b..77f0345 100644 (file)
--- a/core/java/android/app/IActivityManager.java
+++ b/core/java/android/app/IActivityManager.java
@@ -428,7 +428,7 @@ public interface IActivityManager extends IInterface {
     public boolean isTopOfTask(IBinder token) throws RemoteException;
 
     public void crashApplication(int uid, int initialPid, String packageName,
-            String message) throws RemoteException;
+            String message, boolean force) throws RemoteException;
 
     public String getProviderMimeType(Uri uri, int userId) throws RemoteException;
 

diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java
index 60b8623..c38dad9 100755 (executable)
--- a/services/core/java/com/android/server/am/ActiveServices.java
+++ b/services/core/java/com/android/server/am/ActiveServices.java
@@ -618,6 +618,15 @@ public final class ActiveServices {
         }
     }
 
+    void killMisbehavingService(ServiceRecord r,
+            int appUid, int appPid, String localPackageName) {
+        synchronized (mAm) {
+            stopServiceLocked(r);
+            mAm.crashApplication(appUid, appPid, localPackageName,
+                    "Bad notification for startForeground", true /*force*/);
+        }
+    }
+
     IBinder peekServiceLocked(Intent service, String resolvedType, String callingPackage) {
         ServiceLookupResult r = retrieveServiceLocked(service, resolvedType, callingPackage,
                 Binder.getCallingPid(), Binder.getCallingUid(),

diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 72f2c94..8829fd3 100644 (file)
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -5122,7 +5122,7 @@ public final class ActivityManagerService extends ActivityManagerNative
 
     @Override
     public void crashApplication(int uid, int initialPid, String packageName,
-            String message) {
+            String message, boolean force) {
         if (checkCallingPermission(android.Manifest.permission.FORCE_STOP_PACKAGES)
                 != PackageManager.PERMISSION_GRANTED) {
             String msg = "Permission Denial: crashApplication() from pid="
@@ -5134,7 +5134,8 @@ public final class ActivityManagerService extends ActivityManagerNative
         }
 
         synchronized(this) {
-            mAppErrors.scheduleAppCrashLocked(uid, initialPid, packageName, message);
+            mAppErrors.scheduleAppCrashLocked(uid, initialPid, packageName,
+                    message, force);
         }
     }
 

diff --git a/services/core/java/com/android/server/am/AppErrors.java b/services/core/java/com/android/server/am/AppErrors.java
index 0e2fa23..0618395 100644 (file)
--- a/services/core/java/com/android/server/am/AppErrors.java
+++ b/services/core/java/com/android/server/am/AppErrors.java
@@ -242,25 +242,29 @@ class AppErrors {
     }
 
     void killAppAtUserRequestLocked(ProcessRecord app, Dialog fromDialog) {
-        app.crashing = false;
-        app.crashingReport = null;
-        app.notResponding = false;
-        app.notRespondingReport = null;
         if (app.anrDialog == fromDialog) {
             app.anrDialog = null;
         }
         if (app.waitDialog == fromDialog) {
             app.waitDialog = null;
         }
+        killAppImmediateLocked(app, "user-terminated", "user request after error");
+    }
+
+    private void killAppImmediateLocked(ProcessRecord app, String reason, String killReason) {
+        app.crashing = false;
+        app.crashingReport = null;
+        app.notResponding = false;
+        app.notRespondingReport = null;
         if (app.pid > 0 && app.pid != MY_PID) {
-            handleAppCrashLocked(app, "user-terminated" /*reason*/,
+            handleAppCrashLocked(app, reason,
                     null /*shortMsg*/, null /*longMsg*/, null /*stackTrace*/, null /*data*/);
-            app.kill("user request after error", true);
+            app.kill(killReason, true);
         }
     }
 
     void scheduleAppCrashLocked(int uid, int initialPid, String packageName,
-            String message) {
+            String message, boolean force) {
         ProcessRecord proc = null;
 
         // Figure out which process to kill.  We don't trust that initialPid
@@ -291,6 +295,14 @@ class AppErrors {
         }
 
         proc.scheduleCrash(message);
+        if (force) {
+            // If the app is responsive, the scheduled crash will happen as expected
+            // and then the delayed summary kill will be a no-op.
+            final ProcessRecord p = proc;
+            mService.mHandler.postDelayed(
+                    () -> killAppImmediateLocked(p, "forced", "killed for invalid state"),
+                    5000L);
+        }
     }
 
     /**

diff --git a/services/core/java/com/android/server/am/ServiceRecord.java b/services/core/java/com/android/server/am/ServiceRecord.java
index 71c7fd3..3ee73da 100644 (file)
--- a/services/core/java/com/android/server/am/ServiceRecord.java
+++ b/services/core/java/com/android/server/am/ServiceRecord.java
@@ -448,6 +448,7 @@ final class ServiceRecord extends Binder {
             final String localPackageName = packageName;
             final int localForegroundId = foregroundId;
             final Notification _foregroundNoti = foregroundNoti;
+            final ServiceRecord record = this;
             ams.mHandler.post(new Runnable() {
                 public void run() {
                     NotificationManagerInternal nm = LocalServices.getService(
@@ -532,10 +533,8 @@ final class ServiceRecord extends Binder {
                         Slog.w(TAG, "Error showing notification for service", e);
                         // If it gave us a garbage notification, it doesn't
                         // get to be foreground.
-                        ams.setServiceForeground(name, ServiceRecord.this,
-                                0, null, 0);
-                        ams.crashApplication(appUid, appPid, localPackageName,
-                                "Bad notification for startForeground: " + e);
+                        ams.mServices.killMisbehavingService(record,
+                                appUid, appPid, localPackageName);
                     }
                 }
             });

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index 166f02f..4a827dc 100644 (file)
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -639,7 +639,7 @@ public class NotificationManagerService extends SystemService {
             try {
                 ActivityManagerNative.getDefault().crashApplication(uid, initialPid, pkg,
                         "Bad notification posted from package " + pkg
-                        + ": " + message);
+                        + ": " + message, true /*force*/);
             } catch (RemoteException e) {
             } finally {
                 Binder.restoreCallingIdentity(ident);