Merge "Fix issue #62390590: SecurityException in JobIntentService$..." into oc-dev

12 files changed, 86 insertions, 51 deletions

diff --git a/services/core/java/com/android/server/job/GrantedUriPermissions.java b/services/core/java/com/android/server/job/GrantedUriPermissions.java
index e413d8d1d8c0..c23b109bb9de 100644
--- a/services/core/java/com/android/server/job/GrantedUriPermissions.java
+++ b/services/core/java/com/android/server/job/GrantedUriPermissions.java
@@ -29,7 +29,7 @@ import android.util.Slog;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 
-public class GrantedUriPermissions {
+public final class GrantedUriPermissions {
     private final int mGrantFlags;
     private final int mSourceUserId;
     private final String mTag;
diff --git a/services/core/java/com/android/server/job/JobSchedulerShellCommand.java b/services/core/java/com/android/server/job/JobSchedulerShellCommand.java
index 2d2f61f0288d..a53c0885f334 100644
--- a/services/core/java/com/android/server/job/JobSchedulerShellCommand.java
+++ b/services/core/java/com/android/server/job/JobSchedulerShellCommand.java
@@ -26,7 +26,7 @@ import android.os.UserHandle;
 
 import java.io.PrintWriter;
 
-public class JobSchedulerShellCommand extends ShellCommand {
+public final class JobSchedulerShellCommand extends ShellCommand {
     public static final int CMD_ERR_NO_PACKAGE = -1000;
     public static final int CMD_ERR_NO_JOB = -1001;
     public static final int CMD_ERR_CONSTRAINTS = -1002;
diff --git a/services/core/java/com/android/server/job/JobServiceContext.java b/services/core/java/com/android/server/job/JobServiceContext.java
index 637db11af6d8..5d3f6f7b804b 100644
--- a/services/core/java/com/android/server/job/JobServiceContext.java
+++ b/services/core/java/com/android/server/job/JobServiceContext.java
@@ -55,13 +55,13 @@ import com.android.server.job.controllers.JobStatus;
  * job lands, and again when it is complete.
  * - Cancelling is trickier, because there are also interactions from the client. It's possible
  * the {@link com.android.server.job.JobServiceContext.JobServiceHandler} tries to process a
- * {@link #doCancelLocked(int)} after the client has already finished. This is handled by having
+ * {@link #doCancelLocked} after the client has already finished. This is handled by having
  * {@link com.android.server.job.JobServiceContext.JobServiceHandler#handleCancelLocked} check whether
  * the context is still valid.
  * To mitigate this, we avoid sending duplicate onStopJob()
  * calls to the client after they've specified jobFinished().
  */
-public class JobServiceContext extends IJobCallback.Stub implements ServiceConnection {
+public final class JobServiceContext implements ServiceConnection {
     private static final boolean DEBUG = JobSchedulerService.DEBUG;
     private static final String TAG = "JobServiceContext";
     /** Amount of time a job is allowed to execute for before being considered timed-out. */
@@ -112,6 +112,7 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
      * Writes can only be done from the handler thread, or {@link #executeRunnableJob(JobStatus)}.
      */
     private JobStatus mRunningJob;
+    private JobCallback mRunningCallback;
     /** Used to store next job to run when current job is to be preempted. */
     private int mPreferredUid;
     IJobService service;
@@ -133,6 +134,36 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
     // Debugging: time this job was last stopped.
     public long mStoppedTime;
 
+    final class JobCallback extends IJobCallback.Stub {
+        public String mStoppedReason;
+        public long mStoppedTime;
+
+        @Override
+        public void acknowledgeStartMessage(int jobId, boolean ongoing) {
+            doAcknowledgeStartMessage(this, jobId, ongoing);
+        }
+
+        @Override
+        public void acknowledgeStopMessage(int jobId, boolean reschedule) {
+            doAcknowledgeStopMessage(this, jobId, reschedule);
+        }
+
+        @Override
+        public JobWorkItem dequeueWork(int jobId) {
+            return doDequeueWork(this, jobId);
+        }
+
+        @Override
+        public boolean completeWork(int jobId, int workId) {
+            return doCompleteWork(this, jobId, workId);
+        }
+
+        @Override
+        public void jobFinished(int jobId, boolean reschedule) {
+            doJobFinished(this, jobId, reschedule);
+        }
+    }
+
     JobServiceContext(JobSchedulerService service, IBatteryStats batteryStats,
             JobPackageTracker tracker, Looper looper) {
         this(service.getContext(), service.getLock(), batteryStats, tracker, service, looper);
@@ -168,6 +199,7 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
             mPreferredUid = NO_PREFERRED_UID;
 
             mRunningJob = job;
+            mRunningCallback = new JobCallback();
             final boolean isDeadlineExpired =
                     job.hasDeadlineConstraint() &&
                             (job.getLatestRunTimeElapsed() < SystemClock.elapsedRealtime());
@@ -182,7 +214,7 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
                 job.changedAuthorities.toArray(triggeredAuthorities);
             }
             final JobInfo ji = job.getJob();
-            mParams = new JobParameters(this, job.getJobId(), ji.getExtras(),
+            mParams = new JobParameters(mRunningCallback, job.getJobId(), ji.getExtras(),
                     ji.getTransientExtras(), ji.getClipData(), ji.getClipGrantFlags(),
                     isDeadlineExpired, triggeredUris, triggeredAuthorities);
             mExecutionStartTimeElapsed = SystemClock.elapsedRealtime();
@@ -198,6 +230,7 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
                     Slog.d(TAG, job.getServiceComponent().getShortClassName() + " unavailable.");
                 }
                 mRunningJob = null;
+                mRunningCallback = null;
                 mParams = null;
                 mExecutionStartTimeElapsed = 0L;
                 mVerb = VERB_FINISHED;
@@ -263,28 +296,29 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
         return false;
     }
 
-    @Override
-    public void jobFinished(int jobId, boolean reschedule) {
-        doCallback(reschedule, "app called jobFinished");
+    void doJobFinished(JobCallback cb, int jobId, boolean reschedule) {
+        doCallback(cb, reschedule, "app called jobFinished");
     }
 
-    @Override
-    public void acknowledgeStopMessage(int jobId, boolean reschedule) {
-        doCallback(reschedule, null);
+    void doAcknowledgeStopMessage(JobCallback cb, int jobId, boolean reschedule) {
+        doCallback(cb, reschedule, null);
     }
 
-    @Override
-    public void acknowledgeStartMessage(int jobId, boolean ongoing) {
-        doCallback(ongoing, "finished start");
+    void doAcknowledgeStartMessage(JobCallback cb, int jobId, boolean ongoing) {
+        doCallback(cb, ongoing, "finished start");
     }
 
-    @Override
-    public JobWorkItem dequeueWork(int jobId) {
-        final int callingUid = Binder.getCallingUid();
+    JobWorkItem doDequeueWork(JobCallback cb, int jobId) {
         final long ident = Binder.clearCallingIdentity();
         try {
             synchronized (mLock) {
-                assertCallingUidLocked(callingUid);
+                assertCallerLocked(cb);
+                if (mVerb == VERB_STOPPING || mVerb == VERB_FINISHED) {
+                    // This job is either all done, or on its way out.  Either way, it
+                    // should not dispatch any more work.  We will pick up any remaining
+                    // work the next time we start the job again.
+                    return null;
+                }
                 final JobWorkItem work = mRunningJob.dequeueWorkLocked();
                 if (work == null && !mRunningJob.hasExecutingWorkLocked()) {
                     // This will finish the job.
@@ -297,13 +331,11 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
         }
     }
 
-    @Override
-    public boolean completeWork(int jobId, int workId) {
-        final int callingUid = Binder.getCallingUid();
+    boolean doCompleteWork(JobCallback cb, int jobId, int workId) {
         final long ident = Binder.clearCallingIdentity();
         try {
             synchronized (mLock) {
-                assertCallingUidLocked(callingUid);
+                assertCallerLocked(cb);
                 return mRunningJob.completeWorkLocked(ActivityManager.getService(), workId);
             }
         } finally {
@@ -369,8 +401,8 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
      * whether the client exercising the callback is the client we expect.
      * @return True if the binder calling is coming from the client we expect.
      */
-    private boolean verifyCallingUidLocked(final int callingUid) {
-        if (mRunningJob == null || callingUid != mRunningJob.getUid()) {
+    private boolean verifyCallerLocked(JobCallback cb) {
+        if (mRunningCallback != cb) {
             if (DEBUG) {
                 Slog.d(TAG, "Stale callback received, ignoring.");
             }
@@ -379,16 +411,15 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
         return true;
     }
 
-    private void assertCallingUidLocked(final int callingUid) {
-        if (!verifyCallingUidLocked(callingUid)) {
+    private void assertCallerLocked(JobCallback cb) {
+        if (!verifyCallerLocked(cb)) {
             StringBuilder sb = new StringBuilder(128);
-            sb.append("Bad calling uid ");
-            sb.append(callingUid);
-            if (mStoppedReason != null) {
+            sb.append("Caller no longer running");
+            if (cb.mStoppedReason != null) {
                 sb.append(", last stopped ");
-                TimeUtils.formatDuration(SystemClock.elapsedRealtime() - mStoppedTime, sb);
+                TimeUtils.formatDuration(SystemClock.elapsedRealtime() - cb.mStoppedTime, sb);
                 sb.append(" because: ");
-                sb.append(mStoppedReason);
+                sb.append(cb.mStoppedReason);
             }
             throw new SecurityException(sb.toString());
         }
@@ -421,12 +452,11 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
         handleServiceBoundLocked();
     }
 
-    void doCallback(boolean reschedule, String reason) {
-        final int callingUid = Binder.getCallingUid();
+    void doCallback(JobCallback cb, boolean reschedule, String reason) {
         final long ident = Binder.clearCallingIdentity();
         try {
             synchronized (mLock) {
-                if (!verifyCallingUidLocked(callingUid)) {
+                if (!verifyCallerLocked(cb)) {
                     return;
                 }
                 doCallbackLocked(reschedule, reason);
@@ -559,7 +589,7 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
      * VERB_BINDING    -> Cancelled before bind completed. Mark as cancelled and wait for
      *                    {@link #onServiceConnected(android.content.ComponentName, android.os.IBinder)}
      *     _STARTING   -> Mark as cancelled and wait for
-     *                    {@link JobServiceContext#acknowledgeStartMessage(int, boolean)}
+     *                    {@link JobServiceContext#doAcknowledgeStartMessage}
      *     _EXECUTING  -> call {@link #sendStopMessageLocked}}, but only if there are no callbacks
      *                      in the message queue.
      *     _ENDING     -> No point in doing anything here, so we ignore.
@@ -671,6 +701,7 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
         mContext.unbindService(JobServiceContext.this);
         mWakeLock = null;
         mRunningJob = null;
+        mRunningCallback = null;
         mParams = null;
         mVerb = VERB_FINISHED;
         mCancelled = false;
@@ -684,6 +715,10 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne
         if (reason != null && mStoppedReason == null) {
             mStoppedReason = reason;
             mStoppedTime = SystemClock.elapsedRealtime();
+            if (mRunningCallback != null) {
+                mRunningCallback.mStoppedReason = mStoppedReason;
+                mRunningCallback.mStoppedTime = mStoppedTime;
+            }
         }
     }
 
diff --git a/services/core/java/com/android/server/job/JobStore.java b/services/core/java/com/android/server/job/JobStore.java
index 22eed3b04250..f0cd8a8569e0 100644
--- a/services/core/java/com/android/server/job/JobStore.java
+++ b/services/core/java/com/android/server/job/JobStore.java
@@ -66,7 +66,7 @@ import org.xmlpull.v1.XmlSerializer;
  *      and {@link com.android.server.job.JobStore.ReadJobMapFromDiskRunnable} lock on that
  *      object.
  */
-public class JobStore {
+public final class JobStore {
     private static final String TAG = "JobStore";
     private static final boolean DEBUG = JobSchedulerService.DEBUG;
 
@@ -263,7 +263,7 @@ public class JobStore {
      * Runnable that writes {@link #mJobSet} out to xml.
      * NOTE: This Runnable locks on mLock
      */
-    private class WriteJobsMapToDiskRunnable implements Runnable {
+    private final class WriteJobsMapToDiskRunnable implements Runnable {
         @Override
         public void run() {
             final long startElapsed = SystemClock.elapsedRealtime();
@@ -444,7 +444,7 @@ public class JobStore {
      * Runnable that reads list of persisted job from xml. This is run once at start up, so doesn't
      * need to go through {@link JobStore#add(com.android.server.job.controllers.JobStatus)}.
      */
-    private class ReadJobMapFromDiskRunnable implements Runnable {
+    private final class ReadJobMapFromDiskRunnable implements Runnable {
         private final JobSet jobSet;
 
         /**
@@ -796,7 +796,7 @@ public class JobStore {
         }
     }
 
-    static class JobSet {
+    static final class JobSet {
         // Key is the getUid() originator of the jobs in each sheaf
         private SparseArray<ArraySet<JobStatus>> mJobs;
 
diff --git a/services/core/java/com/android/server/job/controllers/AppIdleController.java b/services/core/java/com/android/server/job/controllers/AppIdleController.java
index 68dd00ff00c1..39f2a96b30e3 100644
--- a/services/core/java/com/android/server/job/controllers/AppIdleController.java
+++ b/services/core/java/com/android/server/job/controllers/AppIdleController.java
@@ -33,7 +33,7 @@ import java.io.PrintWriter;
  * for a certain amount of time (maybe hours or days) are considered idle. When the app comes
  * out of idle state, it will be allowed to run scheduled jobs.
  */
-public class AppIdleController extends StateController {
+public final class AppIdleController extends StateController {
 
     private static final String LOG_TAG = "AppIdleController";
     private static final boolean DEBUG = false;
@@ -171,7 +171,7 @@ public class AppIdleController extends StateController {
         }
     }
 
-    private class AppIdleStateChangeListener
+    private final class AppIdleStateChangeListener
             extends UsageStatsManagerInternal.AppIdleStateChangeListener {
         @Override
         public void onAppIdleStateChanged(String packageName, int userId, boolean idle) {
diff --git a/services/core/java/com/android/server/job/controllers/BatteryController.java b/services/core/java/com/android/server/job/controllers/BatteryController.java
index d275bd94a6f4..91119690617a 100644
--- a/services/core/java/com/android/server/job/controllers/BatteryController.java
+++ b/services/core/java/com/android/server/job/controllers/BatteryController.java
@@ -39,7 +39,7 @@ import java.io.PrintWriter;
  * be charging when it's been plugged in for more than two minutes, and the system has broadcast
  * ACTION_BATTERY_OK.
  */
-public class BatteryController extends StateController {
+public final class BatteryController extends StateController {
     private static final String TAG = "JobScheduler.Batt";
 
     private static final Object sCreationLock = new Object();
@@ -121,7 +121,7 @@ public class BatteryController extends StateController {
         }
     }
 
-    public class ChargingTracker extends BroadcastReceiver {
+    public final class ChargingTracker extends BroadcastReceiver {
         /**
          * Track whether we're "charging", where charging means that we're ready to commit to
          * doing work.
diff --git a/services/core/java/com/android/server/job/controllers/ConnectivityController.java b/services/core/java/com/android/server/job/controllers/ConnectivityController.java
index f4268185aa40..17c89282f280 100644
--- a/services/core/java/com/android/server/job/controllers/ConnectivityController.java
+++ b/services/core/java/com/android/server/job/controllers/ConnectivityController.java
@@ -43,7 +43,7 @@ import java.io.PrintWriter;
  * status due to user-requested network policies, so we need to check
  * constraints on a per-UID basis.
  */
-public class ConnectivityController extends StateController implements
+public final class ConnectivityController extends StateController implements
         ConnectivityManager.OnNetworkActiveListener {
     private static final String TAG = "JobScheduler.Conn";
     private static final boolean DEBUG = false;
diff --git a/services/core/java/com/android/server/job/controllers/ContentObserverController.java b/services/core/java/com/android/server/job/controllers/ContentObserverController.java
index cfafc38428f3..ff807eccee7b 100644
--- a/services/core/java/com/android/server/job/controllers/ContentObserverController.java
+++ b/services/core/java/com/android/server/job/controllers/ContentObserverController.java
@@ -39,7 +39,7 @@ import java.util.ArrayList;
 /**
  * Controller for monitoring changes to content URIs through a ContentObserver.
  */
-public class ContentObserverController extends StateController {
+public final class ContentObserverController extends StateController {
     private static final String TAG = "JobScheduler.Content";
     private static final boolean DEBUG = false;
 
diff --git a/services/core/java/com/android/server/job/controllers/DeviceIdleJobsController.java b/services/core/java/com/android/server/job/controllers/DeviceIdleJobsController.java
index 5ccf81288255..85993b900dc9 100644
--- a/services/core/java/com/android/server/job/controllers/DeviceIdleJobsController.java
+++ b/services/core/java/com/android/server/job/controllers/DeviceIdleJobsController.java
@@ -37,7 +37,7 @@ import java.util.Arrays;
  * When device is dozing, set constraint for all jobs, except whitelisted apps, as not satisfied.
  * When device is not dozing, set constraint for all jobs as satisfied.
  */
-public class DeviceIdleJobsController extends StateController {
+public final class DeviceIdleJobsController extends StateController {
 
     private static final String LOG_TAG = "DeviceIdleJobsController";
     private static final boolean LOG_DEBUG = false;
diff --git a/services/core/java/com/android/server/job/controllers/IdleController.java b/services/core/java/com/android/server/job/controllers/IdleController.java
index 7e922930e6e8..9eda046fa1e4 100644
--- a/services/core/java/com/android/server/job/controllers/IdleController.java
+++ b/services/core/java/com/android/server/job/controllers/IdleController.java
@@ -33,7 +33,7 @@ import com.android.server.am.ActivityManagerService;
 import com.android.server.job.JobSchedulerService;
 import com.android.server.job.StateChangedListener;
 
-public class IdleController extends StateController {
+public final class IdleController extends StateController {
     private static final String TAG = "IdleController";
 
     // Policy: we decide that we're "idle" if the device has been unused /
@@ -107,7 +107,7 @@ public class IdleController extends StateController {
         mIdleTracker.startTracking();
     }
 
-    class IdlenessTracker extends BroadcastReceiver {
+    final class IdlenessTracker extends BroadcastReceiver {
         private AlarmManager mAlarm;
         private PendingIntent mIdleTriggerIntent;
         boolean mIdle;
diff --git a/services/core/java/com/android/server/job/controllers/StorageController.java b/services/core/java/com/android/server/job/controllers/StorageController.java
index 4fe8eca54a6e..c24e563948d4 100644
--- a/services/core/java/com/android/server/job/controllers/StorageController.java
+++ b/services/core/java/com/android/server/job/controllers/StorageController.java
@@ -35,7 +35,7 @@ import java.io.PrintWriter;
 /**
  * Simple controller that tracks the status of the device's storage.
  */
-public class StorageController extends StateController {
+public final class StorageController extends StateController {
     private static final String TAG = "JobScheduler.Stor";
 
     private static final Object sCreationLock = new Object();
@@ -112,7 +112,7 @@ public class StorageController extends StateController {
         }
     }
 
-    public class StorageTracker extends BroadcastReceiver {
+    public final class StorageTracker extends BroadcastReceiver {
         /**
          * Track whether storage is low.
          */
diff --git a/services/core/java/com/android/server/job/controllers/TimeController.java b/services/core/java/com/android/server/job/controllers/TimeController.java
index 01c841e2083c..d90699a61928 100644
--- a/services/core/java/com/android/server/job/controllers/TimeController.java
+++ b/services/core/java/com/android/server/job/controllers/TimeController.java
@@ -38,7 +38,7 @@ import java.util.ListIterator;
  * This class sets an alarm for the next expiring job, and determines whether a job's minimum
  * delay has been satisfied.
  */
-public class TimeController extends StateController {
+public final class TimeController extends StateController {
     private static final String TAG = "JobScheduler.Time";
 
     /** Deadline alarm tag for logging purposes */