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:
committed by
Thierry Strudel
parent
01228354e4
commit
bbef697a0b
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user