OSDN Git Service

Fix issue #62390590: SecurityException in JobIntentService$...
authorDianne Hackborn <hackbod@google.com>
Tue, 13 Jun 2017 17:33:19 +0000 (10:33 -0700)
committerDianne Hackborn <hackbod@google.com>
Tue, 13 Jun 2017 17:38:01 +0000 (10:38 -0700)
...JobServiceEngineImpl$WrapperWorkItem.complete

When a job is in the process of stopping a job, we should no
longer allow new work to be dispatched for it.

Also there was an issue with how we determine wether a caller
is valid for the current job -- this was only based on the uid
of the currently running job, but of course that context could
be re-used for a new job of the same uid.  Instead, we now create
a different callback binder for each client, so we can identify
the exact client each time it calls back.  (This also allows us
to hang information about why the job last stopped on that
client state, so we can always report it.)

Finally make a bunch of classes final that should have been.

Test: bit CtsJobSchedulerTestCases:*

Change-Id: I1b00dd2da710414dd2898c4d39a5c528d54b95ea

12 files changed:
services/core/java/com/android/server/job/GrantedUriPermissions.java
services/core/java/com/android/server/job/JobSchedulerShellCommand.java
services/core/java/com/android/server/job/JobServiceContext.java
services/core/java/com/android/server/job/JobStore.java
services/core/java/com/android/server/job/controllers/AppIdleController.java
services/core/java/com/android/server/job/controllers/BatteryController.java
services/core/java/com/android/server/job/controllers/ConnectivityController.java
services/core/java/com/android/server/job/controllers/ContentObserverController.java
services/core/java/com/android/server/job/controllers/DeviceIdleJobsController.java
services/core/java/com/android/server/job/controllers/IdleController.java
services/core/java/com/android/server/job/controllers/StorageController.java
services/core/java/com/android/server/job/controllers/TimeController.java

index e413d8d..c23b109 100644 (file)
@@ -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;
index 2d2f61f..a53c088 100644 (file)
@@ -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;
index 637db11..5d3f6f7 100644 (file)
@@ -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;
+            }
         }
     }
 
index 22eed3b..f0cd8a8 100644 (file)
@@ -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;
 
index 68dd00f..39f2a96 100644 (file)
@@ -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) {
index d275bd9..9111969 100644 (file)
@@ -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.
index f426818..17c8928 100644 (file)
@@ -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;
index cfafc38..ff807ec 100644 (file)
@@ -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;
 
index 5ccf812..85993b9 100644 (file)
@@ -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;
index 7e92293..9eda046 100644 (file)
@@ -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;
index 4fe8eca..c24e563 100644 (file)
@@ -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.
          */
index 01c841e..d90699a 100644 (file)
@@ -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 */