From 6a8a1bf65b5a2ee824051709d607d0f80206a013 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Mon, 5 Jun 2023 19:03:46 +0000 Subject: [PATCH] Ensure apps can't bypass FAS using UIJ. Make sure UIJs are set to stop after an app leaves the TOP state if the user Background Restricted the app. Also remove associated notifications so that users don't get confused about the state of the app. Also modifying IDLE_UIDS_MSG queuing to enable deterministic testing. Bug: 285623302 Test: atest CtsJobSchedulerTestCases:JobThrottlingTest Test: atest CtsJobSchedulerTestCases:NotificationTest Test: atest --rerun-until-failure 25 CtsJobSchedulerTestCases:UserInitiatedJobTest Test: atest FrameworksMockingServicesTests:JobNotificationCoordinatorTest Test: atest FrameworksMockingServicesTests:MockingOomAdjusterTests Test: atest FrameworksServicesTests:OomAdjusterTests Change-Id: I59f9880b0cfc4e5ba143cd522ed4c3df012f7154 --- .../job/JobNotificationCoordinator.java | 3 ++ .../android/server/job/JobServiceContext.java | 13 ++++++-- .../controllers/BackgroundJobsController.java | 32 ++++++++++++++++--- .../controllers/ConnectivityController.java | 6 ++++ .../server/job/controllers/JobStatus.java | 22 +++++++++++++ core/java/android/app/IActivityManager.aidl | 1 + .../server/am/ActivityManagerConstants.java | 6 ++-- .../server/am/ActivityManagerService.java | 7 ++++ .../am/ActivityManagerShellCommand.java | 24 ++++++++++++++ .../com/android/server/am/OomAdjuster.java | 8 +++-- .../com/android/server/am/ProcessList.java | 4 ++- 11 files changed, 113 insertions(+), 13 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobNotificationCoordinator.java b/apex/jobscheduler/service/java/com/android/server/job/JobNotificationCoordinator.java index ae86afbd83800..071707059f2dc 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobNotificationCoordinator.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobNotificationCoordinator.java @@ -200,7 +200,10 @@ class JobNotificationCoordinator { // No more jobs using this notification. Apply the final job stop policy. // If the user attempted to stop the job/app, then always remove the notification // so the user doesn't get confused about the app state. + // Similarly, if the user background restricted the app, remove the notification so + // the user doesn't think the app is continuing to run in the background. if (details.jobEndNotificationPolicy == JOB_END_NOTIFICATION_POLICY_REMOVE + || stopReason == JobParameters.STOP_REASON_BACKGROUND_RESTRICTION || stopReason == JobParameters.STOP_REASON_USER) { mNotificationManagerInternal.cancelNotification( packageName, packageName, details.appUid, details.appPid, /* tag */ null, diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java index 5f795b6fedd9b..109686d76b2ff 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java @@ -413,16 +413,22 @@ public final class JobServiceContext implements ServiceConnection { final Intent intent = new Intent().setComponent(job.getServiceComponent()) .setFlags(Intent.FLAG_FROM_BACKGROUND); boolean binding = false; + boolean startedWithForegroundFlag = false; try { final Context.BindServiceFlags bindFlags; - if (job.shouldTreatAsUserInitiatedJob()) { + if (job.shouldTreatAsUserInitiatedJob() && !job.isUserBgRestricted()) { + // If the user has bg restricted the app, don't give the job FG privileges + // such as bypassing data saver or getting the higher foreground proc state. + // If we've gotten to this point, the app is most likely in the foreground, + // so the job will run just fine while the user keeps the app in the foreground. bindFlags = Context.BindServiceFlags.of( Context.BIND_AUTO_CREATE | Context.BIND_ALMOST_PERCEPTIBLE | Context.BIND_BYPASS_POWER_NETWORK_RESTRICTIONS | Context.BIND_BYPASS_USER_NETWORK_RESTRICTIONS | Context.BIND_NOT_APP_COMPONENT_USAGE); - } else if (job.shouldTreatAsExpeditedJob()) { + startedWithForegroundFlag = true; + } else if (job.shouldTreatAsExpeditedJob() || job.shouldTreatAsUserInitiatedJob()) { bindFlags = Context.BindServiceFlags.of( Context.BIND_AUTO_CREATE | Context.BIND_NOT_FOREGROUND @@ -535,8 +541,11 @@ public final class JobServiceContext implements ServiceConnection { mAvailable = false; mStoppedReason = null; mStoppedTime = 0; + // Wait until after bindService() returns a success value to set these so we don't + // have JobStatus objects that aren't running but have these set to true. job.startedAsExpeditedJob = job.shouldTreatAsExpeditedJob(); job.startedAsUserInitiatedJob = job.shouldTreatAsUserInitiatedJob(); + job.startedWithForegroundFlag = startedWithForegroundFlag; return true; } } diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/BackgroundJobsController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/BackgroundJobsController.java index ecee10a13c1d8..25b3421a55f03 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/BackgroundJobsController.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/BackgroundJobsController.java @@ -19,6 +19,7 @@ package com.android.server.job.controllers; import static com.android.server.job.JobSchedulerService.NEVER_INDEX; import static com.android.server.job.JobSchedulerService.sElapsedRealtimeClock; +import android.app.ActivityManager; import android.app.ActivityManagerInternal; import android.os.SystemClock; import android.os.UserHandle; @@ -205,8 +206,32 @@ public final class BackgroundJobsController extends StateController { final int uid = jobStatus.getSourceUid(); final String packageName = jobStatus.getSourcePackageName(); - final boolean canRun = !mAppStateTracker.areJobsRestricted(uid, packageName, - jobStatus.canRunInBatterySaver()); + final boolean isUserBgRestricted = + !mActivityManagerInternal.isBgAutoRestrictedBucketFeatureFlagEnabled() + && !mAppStateTracker.isRunAnyInBackgroundAppOpsAllowed(uid, packageName); + // If a job started with the foreground flag, it'll cause the UID to stay active + // and thus cause areJobsRestricted() to always return false, so if + // areJobsRestricted() returns false and the app is BG restricted and not TOP, + // we need to stop any jobs that started with the foreground flag so they don't + // keep the app in an elevated proc state. If we were to get in this situation, + // then the user restricted the app after the job started, so it's best to stop + // the job as soon as possible, especially since the job would be visible to the + // user (with a notification and in Task Manager). + // There are several other reasons that uidActive can be true for an app even if its + // proc state is less important than BFGS. + // JobScheduler has historically (at least up through UDC) allowed the app's jobs to run + // when its UID was active, even if it's background restricted. This has been fine because + // JobScheduler stops the job as soon as the UID becomes inactive and the jobs themselves + // will not keep the UID active. The logic here is to ensure that special jobs + // (e.g. user-initiated jobs) themselves do not keep the UID active when the app is + // background restricted. + final boolean shouldStopImmediately = jobStatus.startedWithForegroundFlag + && isUserBgRestricted + && mService.getUidProcState(uid) + > ActivityManager.PROCESS_STATE_BOUND_FOREGROUND_SERVICE; + final boolean canRun = !shouldStopImmediately + && !mAppStateTracker.areJobsRestricted( + uid, packageName, jobStatus.canRunInBatterySaver()); final boolean isActive; if (activeState == UNKNOWN) { @@ -219,8 +244,7 @@ public final class BackgroundJobsController extends StateController { } boolean didChange = jobStatus.setBackgroundNotRestrictedConstraintSatisfied(nowElapsed, canRun, - !mActivityManagerInternal.isBgAutoRestrictedBucketFeatureFlagEnabled() - && !mAppStateTracker.isRunAnyInBackgroundAppOpsAllowed(uid, packageName)); + isUserBgRestricted); didChange |= jobStatus.setUidActive(isActive); return didChange; } diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java index f6bdb9303a040..6d938debde10e 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java @@ -1774,6 +1774,12 @@ public final class ConnectivityController extends RestrictingController implemen } pw.println(); + if (mBackgroundMeteredAllowed.size() > 0) { + pw.print("Background metered allowed: "); + pw.println(mBackgroundMeteredAllowed); + pw.println(); + } + pw.println("Current default network callbacks:"); pw.increaseIndent(); for (int i = 0; i < mCurrentDefaultNetworkCallbacks.size(); i++) { diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java index 3baa9e6d3de95..13903acc04395 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java @@ -430,6 +430,13 @@ public final class JobStatus { * when it started running. This isn't copied over when a job is rescheduled. */ public boolean startedAsUserInitiatedJob = false; + /** + * Whether this particular JobStatus instance started with the foreground flag + * (or more accurately, did not have the + * {@link android.content.Context#BIND_NOT_FOREGROUND} flag + * included in its binding flags when started). + */ + public boolean startedWithForegroundFlag = false; public boolean startedWithImmediacyPrivilege = false; @@ -1606,6 +1613,10 @@ public final class JobStatus { * for any reason. */ public boolean shouldTreatAsUserInitiatedJob() { + // isUserBgRestricted is intentionally excluded from this method. It should be fine to + // treat the job as a UI job while the app is TOP, but just not in the background. + // Instead of adding a proc state check here, the parts of JS that can make the distinction + // and care about the distinction can do the check. return getJob().isUserInitiated() && (getInternalFlags() & INTERNAL_FLAG_DEMOTED_BY_USER) == 0 && (getInternalFlags() & INTERNAL_FLAG_DEMOTED_BY_SYSTEM_UIJ) == 0; @@ -1653,6 +1664,11 @@ public final class JobStatus { && (mDynamicConstraints & CONSTRAINT_BACKGROUND_NOT_RESTRICTED) == 0); } + /** Returns whether or not the app is background restricted by the user (FAS). */ + public boolean isUserBgRestricted() { + return mIsUserBgRestricted; + } + /** @return true if the constraint was changed, false otherwise. */ boolean setChargingConstraintSatisfied(final long nowElapsed, boolean state) { return setConstraintSatisfied(CONSTRAINT_CHARGING, nowElapsed, state); @@ -2802,6 +2818,12 @@ public final class JobStatus { } pw.decreaseIndent(); + pw.print("Started with foreground flag: "); + pw.println(startedWithForegroundFlag); + if (mIsUserBgRestricted) { + pw.println("User BG restricted"); + } + if (changedAuthorities != null) { pw.println("Changed authorities:"); pw.increaseIndent(); diff --git a/core/java/android/app/IActivityManager.aidl b/core/java/android/app/IActivityManager.aidl index 0ec3847d29f46..46260ea5e6586 100644 --- a/core/java/android/app/IActivityManager.aidl +++ b/core/java/android/app/IActivityManager.aidl @@ -607,6 +607,7 @@ interface IActivityManager { void killPackageDependents(in String packageName, int userId); void makePackageIdle(String packageName, int userId); + void setDeterministicUidIdle(boolean deterministic); int getMemoryTrimLevel(); boolean isVrModePackageEnabled(in ComponentName packageName); void notifyLockedProfile(int userId); diff --git a/services/core/java/com/android/server/am/ActivityManagerConstants.java b/services/core/java/com/android/server/am/ActivityManagerConstants.java index 44e198b537617..8c31209aeeb47 100644 --- a/services/core/java/com/android/server/am/ActivityManagerConstants.java +++ b/services/core/java/com/android/server/am/ActivityManagerConstants.java @@ -1678,9 +1678,9 @@ final class ActivityManagerConstants extends ContentObserver { DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, KEY_KILL_BG_RESTRICTED_CACHED_IDLE_SETTLE_TIME, DEFAULT_KILL_BG_RESTRICTED_CACHED_IDLE_SETTLE_TIME_MS); - if (mKillBgRestrictedAndCachedIdleSettleTimeMs != currentSettleTime) { - mService.mHandler.removeMessages( - ActivityManagerService.IDLE_UIDS_MSG); + if (mKillBgRestrictedAndCachedIdleSettleTimeMs < currentSettleTime) { + // Don't remove existing messages in case other IDLE_UIDS_MSG initiators use lower + // delays, but send a new message if the settle time has decreased. mService.mHandler.sendEmptyMessageDelayed( ActivityManagerService.IDLE_UIDS_MSG, mKillBgRestrictedAndCachedIdleSettleTimeMs); diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 92e73f585466e..e9f38f1b4655e 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -1522,6 +1522,8 @@ public class ActivityManagerService extends IActivityManager.Stub */ int mBootPhase; + volatile boolean mDeterministicUidIdle = false; + @VisibleForTesting public WindowManagerService mWindowManager; WindowManagerInternal mWmInternal; @@ -16464,6 +16466,11 @@ public class ActivityManagerService extends IActivityManager.Stub } } + @Override + public void setDeterministicUidIdle(boolean deterministic) { + mDeterministicUidIdle = deterministic; + } + /** Make the currently active UIDs idle after a certain grace period. */ final void idleUids() { synchronized (this) { diff --git a/services/core/java/com/android/server/am/ActivityManagerShellCommand.java b/services/core/java/com/android/server/am/ActivityManagerShellCommand.java index add22bd6009e6..fd980727e12b1 100644 --- a/services/core/java/com/android/server/am/ActivityManagerShellCommand.java +++ b/services/core/java/com/android/server/am/ActivityManagerShellCommand.java @@ -290,6 +290,8 @@ final class ActivityManagerShellCommand extends ShellCommand { return runKillAll(pw); case "make-uid-idle": return runMakeIdle(pw); + case "set-deterministic-uid-idle": + return runSetDeterministicUidIdle(pw); case "monitor": return runMonitor(pw); case "watch-uids": @@ -1520,6 +1522,23 @@ final class ActivityManagerShellCommand extends ShellCommand { return 0; } + int runSetDeterministicUidIdle(PrintWriter pw) throws RemoteException { + int userId = UserHandle.USER_ALL; + + String opt; + while ((opt = getNextOption()) != null) { + if (opt.equals("--user")) { + userId = UserHandle.parseUserArg(getNextArgRequired()); + } else { + getErrPrintWriter().println("Error: Unknown option: " + opt); + return -1; + } + } + boolean deterministic = Boolean.parseBoolean(getNextArgRequired()); + mInterface.setDeterministicUidIdle(deterministic); + return 0; + } + static final class MyActivityController extends IActivityController.Stub { final IActivityManager mInterface; final PrintWriter mPw; @@ -4271,6 +4290,11 @@ final class ActivityManagerShellCommand extends ShellCommand { pw.println(" make-uid-idle [--user | all | current] "); pw.println(" If the given application's uid is in the background and waiting to"); pw.println(" become idle (not allowing background services), do that now."); + pw.println( + " set-deterministic-uid-idle [--user | all | current] "); + pw.println(" If true, sets the timing of making UIDs idle consistent and"); + pw.println(" deterministic. If false, the timing will be variable depending on"); + pw.println(" other activity on the device. The default is false."); pw.println(" monitor [--gdb ] [-p ] [-s] [-c] [-k]"); pw.println(" Start monitoring for crashes or ANRs."); pw.println(" --gdb: start gdbserv on the given port at crash/ANR"); diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java index a86c2e362c544..764bbe8bd191f 100644 --- a/services/core/java/com/android/server/am/OomAdjuster.java +++ b/services/core/java/com/android/server/am/OomAdjuster.java @@ -1471,7 +1471,8 @@ public class OomAdjuster { if (!ActivityManager.isProcStateBackground(uidRec.getSetProcState()) || uidRec.isSetAllowListed()) { uidRec.setLastBackgroundTime(nowElapsed); - if (!mService.mHandler.hasMessages(IDLE_UIDS_MSG)) { + if (mService.mDeterministicUidIdle + || !mService.mHandler.hasMessages(IDLE_UIDS_MSG)) { // Note: the background settle time is in elapsed realtime, while // the handler time base is uptime. All this means is that we may // stop background uids later than we had intended, but that only @@ -3227,7 +3228,8 @@ public class OomAdjuster { // (for states debouncing to avoid from thrashing). state.setLastCanKillOnBgRestrictedAndIdleTime(nowElapsed); // Kick off the delayed checkup message if needed. - if (!mService.mHandler.hasMessages(IDLE_UIDS_MSG)) { + if (mService.mDeterministicUidIdle + || !mService.mHandler.hasMessages(IDLE_UIDS_MSG)) { mService.mHandler.sendEmptyMessageDelayed(IDLE_UIDS_MSG, mConstants.mKillBgRestrictedAndCachedIdleSettleTimeMs); } @@ -3346,6 +3348,7 @@ public class OomAdjuster { @GuardedBy("mService") void idleUidsLocked() { final int N = mActiveUids.size(); + mService.mHandler.removeMessages(IDLE_UIDS_MSG); if (N <= 0) { return; } @@ -3391,7 +3394,6 @@ public class OomAdjuster { } } if (nextTime > 0) { - mService.mHandler.removeMessages(IDLE_UIDS_MSG); mService.mHandler.sendEmptyMessageDelayed(IDLE_UIDS_MSG, nextTime + mConstants.BACKGROUND_SETTLE_TIME - nowElapsed); } diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java index 4342cb9947541..86b09ab41ce9b 100644 --- a/services/core/java/com/android/server/am/ProcessList.java +++ b/services/core/java/com/android/server/am/ProcessList.java @@ -5243,7 +5243,9 @@ public final class ProcessList { mAppsInBackgroundRestricted.add(app); final long future = killAppIfBgRestrictedAndCachedIdleLocked( app, nowElapsed); - if (future > 0 && !mService.mHandler.hasMessages(IDLE_UIDS_MSG)) { + if (future > 0 + && (mService.mDeterministicUidIdle + || !mService.mHandler.hasMessages(IDLE_UIDS_MSG))) { mService.mHandler.sendEmptyMessageDelayed(IDLE_UIDS_MSG, future - nowElapsed); }