From 853f61329ed37c5971613e3c6d6e0b5820aeafa5 Mon Sep 17 00:00:00 2001 From: Hui Yu Date: Mon, 3 Aug 2020 23:13:06 -0700 Subject: [PATCH] Prototyping background started FGS startForeground() restriction. 1. Add a field mAllowStartForeground in ServiceRecord, this field is set to true if the service is started from foreground or meet the exemption conditions (the exemption conditions is the same exemption list as "background started FGS while-in-use permission restriction" in R. 2. Add a DeviceConfig flag "default_fgs_starts_restriction_enabled" to enable the FGS startForeground() restriction. Default is disable. 3. if mAllowStartForeground is false and "default_fgs_starts_restriction_enabled" is true, do not allow startForeground() on this service. 4. if mAllowStartForeground is false, generate a WTF log message in following format: ActivityManager: Background started FGS [callingPackage: com.google.android.music; callingUid: 10155; uidState: FGS; intent: Intent { act=com.google.android.music.leanback.APP_IN_USE cmp=com.google.android.music/.autocache.AutoCacheSchedulingService }] On droidfood devices, these log messages are sent to error_stats table, so we can tell how many apps have background started FGS. Bug: 157473819 Test: adb logcat | grep "Background started FGS" atest cts/tests/app/src/android/app/cts/ActivityManagerFgsBgStartTest.java#testFgsStartFromBG Change-Id: Id122fc3379a8a67287454bc1341521ef84b1c614 --- .../com/android/server/am/ActiveServices.java | 89 +++++++++++++++---- .../server/am/ActivityManagerConstants.java | 23 +++++ .../com/android/server/am/ServiceRecord.java | 10 +++ 3 files changed, 103 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index 3eb26de3a675a..b83aa4fb4d86b 100644 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -694,12 +694,8 @@ public final class ActiveServices { } ComponentName cmp = startServiceInnerLocked(smap, service, r, callerFg, addToStarting); - if (!r.mAllowWhileInUsePermissionInFgs) { - r.mAllowWhileInUsePermissionInFgs = - shouldAllowWhileInUsePermissionInFgsLocked(callingPackage, callingPid, - callingUid, service, r, allowBackgroundActivityStarts); - } - + setFgsRestrictionLocked(callingPackage, callingPid, + callingUid, service, r, allowBackgroundActivityStarts); return cmp; } @@ -1369,6 +1365,7 @@ public final class ActiveServices { + r.shortInstanceName); } } + boolean alreadyStartedOp = false; boolean stopProcStatsOp = false; if (r.fgRequired) { @@ -1415,6 +1412,24 @@ public final class ActiveServices { ignoreForeground = true; } + if (!ignoreForeground) { + if (!r.mAllowStartForeground) { + if (!r.mLoggedInfoAllowStartForeground) { + Slog.wtf(TAG, "Background started FGS " + + r.mInfoAllowStartForeground); + r.mLoggedInfoAllowStartForeground = true; + } + if (mAm.mConstants.mFlagFgsStartRestrictionEnabled) { + Slog.w(TAG, + "Service.startForeground() not allowed due to " + + " mAllowStartForeground false: service " + + r.shortInstanceName); + updateServiceForegroundLocked(r.app, true); + ignoreForeground = true; + } + } + } + // Apps under strict background restrictions simply don't get to have foreground // services, so now that we've enforced the startForegroundService() contract // we only do the machinery of making the service foreground when the app @@ -2067,12 +2082,7 @@ public final class ActiveServices { } } - if (!s.mAllowWhileInUsePermissionInFgs) { - s.mAllowWhileInUsePermissionInFgs = - shouldAllowWhileInUsePermissionInFgsLocked(callingPackage, - callingPid, callingUid, - service, s, false); - } + setFgsRestrictionLocked(callingPackage, callingPid, callingUid, service, s, false); if (s.app != null) { if ((flags&Context.BIND_TREAT_LIKE_ACTIVITY) != 0) { @@ -4840,21 +4850,48 @@ public final class ActiveServices { } /** - * Should allow while-in-use permissions in foreground service or not. - * while-in-use permissions in FGS started from background might be restricted. + * There are two FGS restrictions: + * In R, mAllowWhileInUsePermissionInFgs is to allow while-in-use permissions in foreground + * service or not. while-in-use permissions in FGS started from background might be restricted. + * In S, mAllowStartForeground is to allow FGS to startForeground or not. Service started + * from background may not become a FGS. * @param callingPackage caller app's package name. * @param callingUid caller app's uid. * @param intent intent to start/bind service. * @param r the service to start. * @return true if allow, false otherwise. */ - private boolean shouldAllowWhileInUsePermissionInFgsLocked(String callingPackage, + private void setFgsRestrictionLocked(String callingPackage, int callingPid, int callingUid, Intent intent, ServiceRecord r, boolean allowBackgroundActivityStarts) { - // Is the background FGS start restriction turned on? + // Check DeviceConfig flag. if (!mAm.mConstants.mFlagBackgroundFgsStartRestrictionEnabled) { - return true; + r.mAllowWhileInUsePermissionInFgs = true; } + + if (!r.mAllowWhileInUsePermissionInFgs || !r.mAllowStartForeground) { + final boolean temp = shouldAllowFgsFeatureLocked(callingPackage, callingPid, + callingUid, intent, r, allowBackgroundActivityStarts); + if (!r.mAllowWhileInUsePermissionInFgs) { + r.mAllowWhileInUsePermissionInFgs = temp; + } + if (!r.mAllowStartForeground) { + r.mAllowStartForeground = temp; + } + } + } + + /** + * Should allow FGS feature or not. + * @param callingPackage caller app's package name. + * @param callingUid caller app's uid. + * @param intent intent to start/bind service. + * @param r the service to start. + * @return true if allow, false otherwise. + */ + private boolean shouldAllowFgsFeatureLocked(String callingPackage, + int callingPid, int callingUid, Intent intent, ServiceRecord r, + boolean allowBackgroundActivityStarts) { // Is the allow activity background start flag on? if (allowBackgroundActivityStarts) { return true; @@ -4894,10 +4931,11 @@ public final class ActiveServices { } // Is the calling UID at PROCESS_STATE_TOP or above? - final boolean isCallingUidTopApp = appIsTopLocked(callingUid); - if (isCallingUidTopApp) { + final int uidState = mAm.getUidState(callingUid); + if (uidState <= ActivityManager.PROCESS_STATE_TOP) { return true; } + // Does the calling UID have any visible activity? final boolean isCallingUidVisible = mAm.mAtmInternal.isUidForeground(callingUid); if (isCallingUidVisible) { @@ -4915,6 +4953,19 @@ public final class ActiveServices { if (isDeviceOwner) { return true; } + + final String info = + "[callingPackage: " + callingPackage + + "; callingUid: " + callingUid + + "; uidState: " + ProcessList.makeProcStateString(uidState) + + "; intent: " + intent + + "; targetSdkVersion:" + r.appInfo.targetSdkVersion + + "]"; + if (!info.equals(r.mInfoAllowStartForeground)) { + r.mLoggedInfoAllowStartForeground = false; + r.mInfoAllowStartForeground = info; + } + return false; } } diff --git a/services/core/java/com/android/server/am/ActivityManagerConstants.java b/services/core/java/com/android/server/am/ActivityManagerConstants.java index 775119c18037b..e9539be8b0d52 100644 --- a/services/core/java/com/android/server/am/ActivityManagerConstants.java +++ b/services/core/java/com/android/server/am/ActivityManagerConstants.java @@ -144,6 +144,13 @@ final class ActivityManagerConstants extends ContentObserver { private static final String KEY_DEFAULT_BACKGROUND_FGS_STARTS_RESTRICTION_ENABLED = "default_background_fgs_starts_restriction_enabled"; + /** + * Default value for mFlagFgsStartRestrictionEnabled if not explicitly set in + * Settings.Global. + */ + private static final String KEY_DEFAULT_FGS_STARTS_RESTRICTION_ENABLED = + "default_fgs_starts_restriction_enabled"; + // Maximum number of cached processes we will allow. public int MAX_CACHED_PROCESSES = DEFAULT_MAX_CACHED_PROCESSES; @@ -293,6 +300,11 @@ final class ActivityManagerConstants extends ContentObserver { // started, the restriction is on while-in-use permissions.) volatile boolean mFlagBackgroundFgsStartRestrictionEnabled = true; + // Indicates whether the foreground service background start restriction is enabled. + // When the restriction is enabled, service is not allowed to startForeground from background + // at all. + volatile boolean mFlagFgsStartRestrictionEnabled = false; + private final ActivityManagerService mService; private ContentResolver mResolver; private final KeyValueListParser mParser = new KeyValueListParser(','); @@ -436,6 +448,9 @@ final class ActivityManagerConstants extends ContentObserver { case KEY_DEFAULT_BACKGROUND_FGS_STARTS_RESTRICTION_ENABLED: updateBackgroundFgsStartsRestriction(); break; + case KEY_DEFAULT_FGS_STARTS_RESTRICTION_ENABLED: + updateFgsStartsRestriction(); + break; case KEY_OOMADJ_UPDATE_POLICY: updateOomAdjUpdatePolicy(); break; @@ -659,6 +674,7 @@ final class ActivityManagerConstants extends ContentObserver { mFlagForegroundServiceStartsLoggingEnabled = Settings.Global.getInt(mResolver, Settings.Global.FOREGROUND_SERVICE_STARTS_LOGGING_ENABLED, 1) == 1; } + private void updateBackgroundFgsStartsRestriction() { mFlagBackgroundFgsStartRestrictionEnabled = DeviceConfig.getBoolean( DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, @@ -666,6 +682,13 @@ final class ActivityManagerConstants extends ContentObserver { /*defaultValue*/ true); } + private void updateFgsStartsRestriction() { + mFlagFgsStartRestrictionEnabled = DeviceConfig.getBoolean( + DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, + KEY_DEFAULT_FGS_STARTS_RESTRICTION_ENABLED, + /*defaultValue*/ false); + } + private void updateOomAdjUpdatePolicy() { OOMADJ_UPDATE_QUICK = DeviceConfig.getInt( DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, diff --git a/services/core/java/com/android/server/am/ServiceRecord.java b/services/core/java/com/android/server/am/ServiceRecord.java index 4a2703056871c..66677b67b6aab 100644 --- a/services/core/java/com/android/server/am/ServiceRecord.java +++ b/services/core/java/com/android/server/am/ServiceRecord.java @@ -146,6 +146,12 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN // the most recent package that start/bind this service. String mRecentCallingPackage; + // allow the service becomes foreground service? Service started from background may not be + // allowed to become a foreground service. + boolean mAllowStartForeground; + String mInfoAllowStartForeground; + boolean mLoggedInfoAllowStartForeground; + String stringName; // caching of toString private int lastStartId; // identifier of most recent start request. @@ -408,6 +414,10 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN pw.println(mAllowWhileInUsePermissionInFgs); pw.print(prefix); pw.print("recentCallingPackage="); pw.println(mRecentCallingPackage); + pw.print(prefix); pw.print("allowStartForeground="); + pw.println(mAllowStartForeground); + pw.print(prefix); pw.print("infoAllowStartForeground="); + pw.println(mInfoAllowStartForeground); if (delayed) { pw.print(prefix); pw.print("delayed="); pw.println(delayed); }