Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6466c1cc authored by Dianne Hackborn's avatar Dianne Hackborn
Browse files

Fix issue #62390590: SecurityException in JobIntentService$...

...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
parent 4eb76b2d
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -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;
+1 −1
Original line number Diff line number Diff line
@@ -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;
+68 −33
Original line number Diff line number Diff line
@@ -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;
            }
        }
    }

+4 −4
Original line number Diff line number Diff line
@@ -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;

+2 −2
Original line number Diff line number Diff line
@@ -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) {
Loading