binder: protect enqueuing of death notifications

Death notifications weren't protected against being
enqueued simultaneously by two threads. Serialize
with proc lock.

Bug: 36371104
Test: tested manually
Change-Id: Ie0f04985b765c8dc1a69362f1609aa373806fb4c
Signed-off-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Siqi Lin <siqilin@google.com>
This commit is contained in:
Todd Kjos
2017-03-16 16:34:10 -07:00
committed by Thierry Strudel
parent 01228354e4
commit bbef697a0b

View File

@@ -2565,6 +2565,7 @@ static int binder_thread_write(struct binder_proc *proc,
binder_uintptr_t cookie;
struct binder_ref *ref;
struct binder_ref_death *death;
bool do_wakeup = false;
if (get_user(target, (uint32_t __user *)ptr))
return -EFAULT;
@@ -2613,8 +2614,10 @@ static int binder_thread_write(struct binder_proc *proc,
binder_stats_created(BINDER_STAT_DEATH);
INIT_LIST_HEAD(&death->work.entry);
death->cookie = cookie;
binder_proc_lock(proc, __LINE__);
ref->death = death;
if (ref->node->is_zombie) {
if (ref->node->is_zombie &&
list_empty(&ref->death->work.entry)) {
ref->death->work.type = BINDER_WORK_DEAD_BINDER;
if (thread->looper &
(BINDER_LOOPER_STATE_REGISTERED |
@@ -2628,10 +2631,10 @@ static int binder_thread_write(struct binder_proc *proc,
&ref->death->work,
&proc->todo,
__LINE__);
wake_up_interruptible(
&proc->wait);
do_wakeup = true;
}
}
binder_proc_unlock(proc, __LINE__);
} else {
if (ref->death == NULL) {
binder_user_error("%d:%d BC_CLEAR_DEATH_NOTIFICATION death notification not active\n",
@@ -2648,6 +2651,7 @@ static int binder_thread_write(struct binder_proc *proc,
binder_put_ref(ref);
break;
}
binder_proc_lock(proc, __LINE__);
ref->death = NULL;
if (list_empty(&death->work.entry)) {
death->work.type = BINDER_WORK_CLEAR_DEATH_NOTIFICATION;
@@ -2663,20 +2667,23 @@ static int binder_thread_write(struct binder_proc *proc,
&death->work,
&proc->todo,
__LINE__);
wake_up_interruptible(
&proc->wait);
do_wakeup = true;
}
} else {
BUG_ON(death->work.type != BINDER_WORK_DEAD_BINDER);
death->work.type = BINDER_WORK_DEAD_BINDER_AND_CLEAR;
}
binder_proc_unlock(proc, __LINE__);
}
if (do_wakeup)
wake_up_interruptible(&proc->wait);
binder_put_ref(ref);
} break;
case BC_DEAD_BINDER_DONE: {
struct binder_work *w;
binder_uintptr_t cookie;
struct binder_ref_death *death = NULL;
bool do_wakeup = false;
if (get_user(cookie, (binder_uintptr_t __user *)ptr))
return -EFAULT;
@@ -2705,6 +2712,7 @@ static int binder_thread_write(struct binder_proc *proc,
proc->pid, thread->pid, (u64)cookie);
break;
}
binder_proc_lock(proc, __LINE__);
binder_dequeue_work(&death->work, __LINE__);
if (death->work.type == BINDER_WORK_DEAD_BINDER_AND_CLEAR) {
death->work.type = BINDER_WORK_CLEAR_DEATH_NOTIFICATION;
@@ -2718,9 +2726,12 @@ static int binder_thread_write(struct binder_proc *proc,
binder_enqueue_work(&death->work,
&proc->todo,
__LINE__);
wake_up_interruptible(&proc->wait);
do_wakeup = true;
}
}
binder_proc_unlock(proc, __LINE__);
if (do_wakeup)
wake_up_interruptible(&proc->wait);
} break;
default:
@@ -3038,14 +3049,17 @@ retry:
"BR_CLEAR_DEATH_NOTIFICATION_DONE",
(u64)death->cookie);
binder_proc_lock(proc, __LINE__);
if (w->type == BINDER_WORK_CLEAR_DEATH_NOTIFICATION) {
binder_dequeue_work(w, __LINE__);
binder_proc_unlock(proc, __LINE__);
kfree(death);
binder_stats_deleted(BINDER_STAT_DEATH);
} else {
binder_dequeue_work(w, __LINE__);
binder_enqueue_work(w, &proc->delivered_death,
__LINE__);
binder_proc_unlock(proc, __LINE__);
}
binder_unfreeze_worklist(wlist);
@@ -3973,6 +3987,7 @@ static int binder_node_release(struct binder_node *node, int refs)
w = list_first_entry(&tmplist.list, struct binder_work, entry);
death = container_of(w, struct binder_ref_death, work);
binder_proc_lock(proc, __LINE__);
binder_dequeue_work(w, __LINE__);
/*
* It's not safe to touch death after
@@ -3982,6 +3997,7 @@ static int binder_node_release(struct binder_node *node, int refs)
wait = &death->wait_proc->wait;
binder_enqueue_work(w, &death->wait_proc->todo,
__LINE__);
binder_proc_unlock(proc, __LINE__);
wake_up_interruptible(wait);
}
binder_debug(BINDER_DEBUG_DEAD_BINDER,