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

Commit 0cc7654e authored by Matthew Williams's avatar Matthew Williams
Browse files

Race condition in JobServiceContext 

BUG: 23981171
JobServiceContext has a reference to the running job which
1) needs to be copied before providing access to outside callers
2) was not guarded by a lock in one case which might result in an NPE

Change-Id: I7eb04052f3fe63e7b386c564a6bdebf9144e976a
parent 5419280f

services/core/java/com/android/server/job/JobServiceContext.java

+26 −10
Original line number Diff line number Diff line
@@ -109,7 +109,13 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne 
    int mVerb;

    private AtomicBoolean mCancelled = new AtomicBoolean();



    /** All the information maintained about the job currently being executed. */

    /**

     * All the information maintained about the job currently being executed.

     *

     * Any reads (dereferences) not done from the handler thread must be synchronized on

     * {@link #mLock}.

     * Writes can only be done from the handler thread, or {@link #executeRunnableJob(JobStatus)}.

     */

    private JobStatus mRunningJob;

    /** Binder to the client service. */

    IJobService service;
@@ -194,7 +200,8 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne 
     */

    JobStatus getRunningJob() {

        synchronized (mLock) {

            return mRunningJob;

            return mRunningJob == null ?

                    null : new JobStatus(mRunningJob);

        }

    }
@@ -255,15 +262,22 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne 
     */

    @Override

    public void onServiceConnected(ComponentName name, IBinder service) {

        if (!name.equals(mRunningJob.getServiceComponent())) {

        JobStatus runningJob;

        synchronized (mLock) {

            // This isn't strictly necessary b/c the JobServiceHandler is running on the main

            // looper and at this point we can't get any binder callbacks from the client. Better

            // safe than sorry.

            runningJob = mRunningJob;

        }

        if (runningJob == null || !name.equals(runningJob.getServiceComponent())) {

            mCallbackHandler.obtainMessage(MSG_SHUTDOWN_EXECUTION).sendToTarget();

            return;

        }

        this.service = IJobService.Stub.asInterface(service);

        final PowerManager pm =

                (PowerManager) mContext.getSystemService(Context.POWER_SERVICE);

        mWakeLock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, mRunningJob.getTag());

        mWakeLock.setWorkSource(new WorkSource(mRunningJob.getUid()));

        mWakeLock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, runningJob.getTag());

        mWakeLock.setWorkSource(new WorkSource(runningJob.getUid()));

        mWakeLock.setReferenceCounted(false);

        mWakeLock.acquire();

        mCallbackHandler.obtainMessage(MSG_SERVICE_BOUND).sendToTarget();
@@ -281,6 +295,7 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne 
     * @return True if the binder calling is coming from the client we expect.

     */

    private boolean verifyCallingUid() {

        synchronized (mLock) {

            if (mRunningJob == null || Binder.getCallingUid() != mRunningJob.getUid()) {

                if (DEBUG) {

                    Slog.d(TAG, "Stale callback received, ignoring.");
@@ -289,6 +304,7 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne 
            }

            return true;

        }

    }



    /**

     * Handles the lifecycle of the JobService binding/callbacks, etc. The convention within this

services/core/java/com/android/server/job/controllers/JobStatus.java

+7 −0
Original line number Diff line number Diff line
@@ -82,6 +82,13 @@ public class JobStatus { 
        this.numFailures = numFailures;

    }



    /** Copy constructor. */

    public JobStatus(JobStatus jobStatus) {

        this(jobStatus.getJob(), jobStatus.getUid(), jobStatus.getNumFailures());

        this.earliestRunTimeElapsedMillis = jobStatus.getEarliestRunTime();

        this.latestRunTimeElapsedMillis = jobStatus.getLatestRunTimeElapsed();

    }



    /** Create a newly scheduled job. */

    public JobStatus(JobInfo job, int uId) {

        this(job, uId, 0);