FROMLIST: binder: use inner lock to protect thread accounting
authorTodd Kjos <tkjos@google.com>
Fri, 26 May 2017 00:35:02 +0000 (17:35 -0700)
committerTodd Kjos <tkjos@google.com>
Thu, 13 Jul 2017 15:34:22 +0000 (08:34 -0700)
(from https://patchwork.kernel.org/patch/9817763/)

Use the inner lock to protect thread accounting fields in
proc structure: max_threads, requested_threads,
requested_threads_started and ready_threads.

Change-Id: I8cf519f40ddfe4fd00d99b82fdb88dc069611787
Signed-off-by: Todd Kjos <tkjos@google.com>
drivers/android/binder.c

index 964a80997908b927dc93bbd134dd4c6954f9b70f..50831c7dba55ba3e70cb66ae273cbe49aef6f658 100644 (file)
@@ -497,11 +497,15 @@ enum binder_deferred_state {
  * @delivered_death:      list of delivered death notification
  *                        (protected by @inner_lock)
  * @max_threads:          cap on number of binder threads
+ *                        (protected by @inner_lock)
  * @requested_threads:    number of binder threads requested but not
  *                        yet started. In current implementation, can
  *                        only be 0 or 1.
+ *                        (protected by @inner_lock)
  * @requested_threads_started: number binder threads started
+ *                        (protected by @inner_lock)
  * @ready_threads:        number of threads waiting for proc work
+ *                        (protected by @inner_lock)
  * @tmp_ref:              temporary reference to indicate proc is in use
  *                        (protected by @inner_lock)
  * @default_priority:     default scheduler priority
@@ -3234,6 +3238,7 @@ static int binder_thread_write(struct binder_proc *proc,
                        binder_debug(BINDER_DEBUG_THREADS,
                                     "%d:%d BC_REGISTER_LOOPER\n",
                                     proc->pid, thread->pid);
+                       binder_inner_proc_lock(proc);
                        if (thread->looper & BINDER_LOOPER_STATE_ENTERED) {
                                thread->looper |= BINDER_LOOPER_STATE_INVALID;
                                binder_user_error("%d:%d ERROR: BC_REGISTER_LOOPER called after BC_ENTER_LOOPER\n",
@@ -3247,6 +3252,7 @@ static int binder_thread_write(struct binder_proc *proc,
                                proc->requested_threads_started++;
                        }
                        thread->looper |= BINDER_LOOPER_STATE_REGISTERED;
+                       binder_inner_proc_unlock(proc);
                        break;
                case BC_ENTER_LOOPER:
                        binder_debug(BINDER_DEBUG_THREADS,
@@ -3523,11 +3529,11 @@ retry:
        binder_inner_proc_lock(proc);
        wait_for_proc_work = thread->transaction_stack == NULL &&
                binder_worklist_empty_ilocked(&thread->todo);
+       if (wait_for_proc_work)
+               proc->ready_threads++;
        binder_inner_proc_unlock(proc);
 
        thread->looper |= BINDER_LOOPER_STATE_WAITING;
-       if (wait_for_proc_work)
-               proc->ready_threads++;
 
        binder_unlock(__func__);
 
@@ -3558,8 +3564,10 @@ retry:
 
        binder_lock(__func__);
 
+       binder_inner_proc_lock(proc);
        if (wait_for_proc_work)
                proc->ready_threads--;
+       binder_inner_proc_unlock(proc);
        thread->looper &= ~BINDER_LOOPER_STATE_WAITING;
 
        if (ret)
@@ -3849,19 +3857,22 @@ retry:
 done:
 
        *consumed = ptr - buffer;
+       binder_inner_proc_lock(proc);
        if (proc->requested_threads + proc->ready_threads == 0 &&
            proc->requested_threads_started < proc->max_threads &&
            (thread->looper & (BINDER_LOOPER_STATE_REGISTERED |
             BINDER_LOOPER_STATE_ENTERED)) /* the user-space code fails to */
             /*spawn a new thread if we leave this out */) {
                proc->requested_threads++;
+               binder_inner_proc_unlock(proc);
                binder_debug(BINDER_DEBUG_THREADS,
                             "%d:%d BR_SPAWN_LOOPER\n",
                             proc->pid, thread->pid);
                if (put_user(BR_SPAWN_LOOPER, (uint32_t __user *)buffer))
                        return -EFAULT;
                binder_stat_br(proc, thread, BR_SPAWN_LOOPER);
-       }
+       } else
+               binder_inner_proc_unlock(proc);
        return 0;
 }
 
@@ -4241,12 +4252,19 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
                if (ret)
                        goto err;
                break;
-       case BINDER_SET_MAX_THREADS:
-               if (copy_from_user(&proc->max_threads, ubuf, sizeof(proc->max_threads))) {
+       case BINDER_SET_MAX_THREADS: {
+               int max_threads;
+
+               if (copy_from_user(&max_threads, ubuf,
+                                  sizeof(max_threads))) {
                        ret = -EINVAL;
                        goto err;
                }
+               binder_inner_proc_lock(proc);
+               proc->max_threads = max_threads;
+               binder_inner_proc_unlock(proc);
                break;
+       }
        case BINDER_SET_CONTEXT_MGR:
                ret = binder_ioctl_set_ctx_mgr(filp);
                if (ret)