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 Change-Id: I94d9de50bb03c33666471e3dbd9c721e9278f7cb Merged-In: I94d9de50bb03c33666471e3dbd9c721e9278f7cb
This commit is contained in:
@@ -278,7 +278,8 @@ interface IActivityManager {
|
|||||||
boolean isImmersive(in IBinder token);
|
boolean isImmersive(in IBinder token);
|
||||||
void setImmersive(in IBinder token, boolean immersive);
|
void setImmersive(in IBinder token, boolean immersive);
|
||||||
boolean isTopActivityImmersive();
|
boolean isTopActivityImmersive();
|
||||||
void crashApplication(int uid, int initialPid, in String packageName, int userId, in String message);
|
void crashApplication(int uid, int initialPid, in String packageName, int userId,
|
||||||
|
in String message, boolean force);
|
||||||
String getProviderMimeType(in Uri uri, int userId);
|
String getProviderMimeType(in Uri uri, int userId);
|
||||||
IBinder newUriPermissionOwner(in String name);
|
IBinder newUriPermissionOwner(in String name);
|
||||||
void grantUriPermissionFromOwner(in IBinder owner, int fromUid, in String targetPkg,
|
void grantUriPermissionFromOwner(in IBinder owner, int fromUid, in String targetPkg,
|
||||||
|
|||||||
@@ -787,6 +787,15 @@ public final class ActiveServices {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void killMisbehavingService(ServiceRecord r,
|
||||||
|
int appUid, int appPid, String localPackageName) {
|
||||||
|
synchronized (mAm) {
|
||||||
|
stopServiceLocked(r);
|
||||||
|
mAm.crashApplication(appUid, appPid, localPackageName, -1,
|
||||||
|
"Bad notification for startForeground", true /*force*/);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
IBinder peekServiceLocked(Intent service, String resolvedType, String callingPackage) {
|
IBinder peekServiceLocked(Intent service, String resolvedType, String callingPackage) {
|
||||||
ServiceLookupResult r = retrieveServiceLocked(service, resolvedType, callingPackage,
|
ServiceLookupResult r = retrieveServiceLocked(service, resolvedType, callingPackage,
|
||||||
Binder.getCallingPid(), Binder.getCallingUid(),
|
Binder.getCallingPid(), Binder.getCallingUid(),
|
||||||
@@ -3655,7 +3664,7 @@ public final class ActiveServices {
|
|||||||
void serviceForegroundCrash(ProcessRecord app, CharSequence serviceRecord) {
|
void serviceForegroundCrash(ProcessRecord app, CharSequence serviceRecord) {
|
||||||
mAm.crashApplication(app.uid, app.pid, app.info.packageName, app.userId,
|
mAm.crashApplication(app.uid, app.pid, app.info.packageName, app.userId,
|
||||||
"Context.startForegroundService() did not then call Service.startForeground(): "
|
"Context.startForegroundService() did not then call Service.startForeground(): "
|
||||||
+ serviceRecord);
|
+ serviceRecord, false /*force*/);
|
||||||
}
|
}
|
||||||
|
|
||||||
void scheduleServiceTimeoutLocked(ProcessRecord proc) {
|
void scheduleServiceTimeoutLocked(ProcessRecord proc) {
|
||||||
|
|||||||
@@ -5773,7 +5773,7 @@ public class ActivityManagerService extends IActivityManager.Stub
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void crashApplication(int uid, int initialPid, String packageName, int userId,
|
public void crashApplication(int uid, int initialPid, String packageName, int userId,
|
||||||
String message) {
|
String message, boolean force) {
|
||||||
if (checkCallingPermission(android.Manifest.permission.FORCE_STOP_PACKAGES)
|
if (checkCallingPermission(android.Manifest.permission.FORCE_STOP_PACKAGES)
|
||||||
!= PackageManager.PERMISSION_GRANTED) {
|
!= PackageManager.PERMISSION_GRANTED) {
|
||||||
String msg = "Permission Denial: crashApplication() from pid="
|
String msg = "Permission Denial: crashApplication() from pid="
|
||||||
@@ -5785,7 +5785,8 @@ public class ActivityManagerService extends IActivityManager.Stub
|
|||||||
}
|
}
|
||||||
|
|
||||||
synchronized(this) {
|
synchronized(this) {
|
||||||
mAppErrors.scheduleAppCrashLocked(uid, initialPid, packageName, userId, message);
|
mAppErrors.scheduleAppCrashLocked(uid, initialPid, packageName, userId,
|
||||||
|
message, force);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -987,7 +987,7 @@ final class ActivityManagerShellCommand extends ShellCommand {
|
|||||||
} catch (NumberFormatException e) {
|
} catch (NumberFormatException e) {
|
||||||
packageName = arg;
|
packageName = arg;
|
||||||
}
|
}
|
||||||
mInterface.crashApplication(-1, pid, packageName, userId, "shell-induced crash");
|
mInterface.crashApplication(-1, pid, packageName, userId, "shell-induced crash", false);
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -314,20 +314,24 @@ class AppErrors {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void killAppAtUserRequestLocked(ProcessRecord app, Dialog fromDialog) {
|
void killAppAtUserRequestLocked(ProcessRecord app, Dialog fromDialog) {
|
||||||
app.crashing = false;
|
|
||||||
app.crashingReport = null;
|
|
||||||
app.notResponding = false;
|
|
||||||
app.notRespondingReport = null;
|
|
||||||
if (app.anrDialog == fromDialog) {
|
if (app.anrDialog == fromDialog) {
|
||||||
app.anrDialog = null;
|
app.anrDialog = null;
|
||||||
}
|
}
|
||||||
if (app.waitDialog == fromDialog) {
|
if (app.waitDialog == fromDialog) {
|
||||||
app.waitDialog = null;
|
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) {
|
if (app.pid > 0 && app.pid != MY_PID) {
|
||||||
handleAppCrashLocked(app, "user-terminated" /*reason*/,
|
handleAppCrashLocked(app, reason,
|
||||||
null /*shortMsg*/, null /*longMsg*/, null /*stackTrace*/, null /*data*/);
|
null /*shortMsg*/, null /*longMsg*/, null /*stackTrace*/, null /*data*/);
|
||||||
app.kill("user request after error", true);
|
app.kill(killReason, true);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -341,7 +345,7 @@ class AppErrors {
|
|||||||
* @param message
|
* @param message
|
||||||
*/
|
*/
|
||||||
void scheduleAppCrashLocked(int uid, int initialPid, String packageName, int userId,
|
void scheduleAppCrashLocked(int uid, int initialPid, String packageName, int userId,
|
||||||
String message) {
|
String message, boolean force) {
|
||||||
ProcessRecord proc = null;
|
ProcessRecord proc = null;
|
||||||
|
|
||||||
// Figure out which process to kill. We don't trust that initialPid
|
// Figure out which process to kill. We don't trust that initialPid
|
||||||
@@ -374,6 +378,14 @@ class AppErrors {
|
|||||||
}
|
}
|
||||||
|
|
||||||
proc.scheduleCrash(message);
|
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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -584,6 +584,7 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN
|
|||||||
final String localPackageName = packageName;
|
final String localPackageName = packageName;
|
||||||
final int localForegroundId = foregroundId;
|
final int localForegroundId = foregroundId;
|
||||||
final Notification _foregroundNoti = foregroundNoti;
|
final Notification _foregroundNoti = foregroundNoti;
|
||||||
|
final ServiceRecord record = this;
|
||||||
ams.mHandler.post(new Runnable() {
|
ams.mHandler.post(new Runnable() {
|
||||||
public void run() {
|
public void run() {
|
||||||
NotificationManagerInternal nm = LocalServices.getService(
|
NotificationManagerInternal nm = LocalServices.getService(
|
||||||
@@ -682,10 +683,8 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN
|
|||||||
Slog.w(TAG, "Error showing notification for service", e);
|
Slog.w(TAG, "Error showing notification for service", e);
|
||||||
// If it gave us a garbage notification, it doesn't
|
// If it gave us a garbage notification, it doesn't
|
||||||
// get to be foreground.
|
// get to be foreground.
|
||||||
ams.setServiceForeground(name, ServiceRecord.this,
|
ams.mServices.killMisbehavingService(record,
|
||||||
0, null, 0);
|
appUid, appPid, localPackageName);
|
||||||
ams.crashApplication(appUid, appPid, localPackageName, -1,
|
|
||||||
"Bad notification for startForeground: " + e);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -798,8 +798,22 @@ public class NotificationManagerService extends SystemService {
|
|||||||
@Override
|
@Override
|
||||||
public void onNotificationError(int callingUid, int callingPid, String pkg, String tag, int id,
|
public void onNotificationError(int callingUid, int callingPid, String pkg, String tag, int id,
|
||||||
int uid, int initialPid, String message, int userId) {
|
int uid, int initialPid, String message, int userId) {
|
||||||
|
final boolean fgService;
|
||||||
|
synchronized (mNotificationLock) {
|
||||||
|
NotificationRecord r = findNotificationLocked(pkg, tag, id, userId);
|
||||||
|
fgService = r != null && (r.getNotification().flags & FLAG_FOREGROUND_SERVICE) != 0;
|
||||||
|
}
|
||||||
cancelNotification(callingUid, callingPid, pkg, tag, id, 0, 0, false, userId,
|
cancelNotification(callingUid, callingPid, pkg, tag, id, 0, 0, false, userId,
|
||||||
REASON_ERROR, null);
|
REASON_ERROR, null);
|
||||||
|
if (fgService) {
|
||||||
|
// Still crash for foreground services, preventing the not-crash behaviour abused
|
||||||
|
// by apps to give us a garbage notification and silently start a fg service.
|
||||||
|
Binder.withCleanCallingIdentity(
|
||||||
|
() -> mAm.crashApplication(uid, initialPid, pkg, -1,
|
||||||
|
"Bad notification(tag=" + tag + ", id=" + id + ") posted from package "
|
||||||
|
+ pkg + ", crashing app(uid=" + uid + ", pid=" + initialPid + "): "
|
||||||
|
+ message, true /* force */));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|||||||
Reference in New Issue
Block a user