moonjuice/kernel_arpi
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

UPSTREAM: binder: fix UAF when releasing todo list

Browse Source 
When releasing a thread todo list when tearing down
a binder_proc, the following race was possible which
could result in a use-after-free:

1.  Thread 1: enter binder_release_work from binder_thread_release
2.  Thread 2: binder_update_ref_for_handle() -> binder_dec_node_ilocked()
3.  Thread 2: dec nodeA --> 0 (will free node)
4.  Thread 1: ACQ inner_proc_lock
5.  Thread 2: block on inner_proc_lock
6.  Thread 1: dequeue work (BINDER_WORK_NODE, part of nodeA)
7.  Thread 1: REL inner_proc_lock
8.  Thread 2: ACQ inner_proc_lock
9.  Thread 2: todo list cleanup, but work was already dequeued
10. Thread 2: free node
11. Thread 2: REL inner_proc_lock
12. Thread 1: deref w->type (UAF)

The problem was that for a BINDER_WORK_NODE, the binder_work element
must not be accessed after releasing the inner_proc_lock while
processing the todo list elements since another thread might be
handling a deref on the node containing the binder_work element
leading to the node being freed.

Signed-off-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20201009232455.4054810-1-tkjos@google.com
Cc: <stable@vger.kernel.org> # 4.14, 4.19, 5.4, 5.8
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit f3277cbfba)
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Change-Id: I7c1bf0b74824f272664e76206c5dc3b66b9eeaff
This commit is contained in:
Todd Kjos
2020-10-09 16:24:55 -07:00
committed by Greg Kroah-Hartman
parent 8540985fd2
commit 8d3d29f072
1 changed files with 10 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
drivers/android/binder.c
View File

@@ -230,7 +230,7 @@ static struct binder_transaction_log_entry *binder_transaction_log_add(
struct binder_work { struct binder_work {
struct list_head entry; struct list_head entry;
enum { enum binder_work_type {
BINDER_WORK_TRANSACTION = 1, BINDER_WORK_TRANSACTION = 1,
BINDER_WORK_TRANSACTION_COMPLETE, BINDER_WORK_TRANSACTION_COMPLETE,
BINDER_WORK_RETURN_ERROR, BINDER_WORK_RETURN_ERROR,
@@ -917,27 +917,6 @@ static struct binder_work *binder_dequeue_work_head_ilocked(
return w; return w;
} }
/**
* binder_dequeue_work_head() - Dequeues the item at head of list
* @proc: binder_proc associated with list
* @list: list to dequeue head
*
* Removes the head of the list if there are items on the list
*
* Return: pointer dequeued binder_work, NULL if list was empty
*/
static struct binder_work *binder_dequeue_work_head(
struct binder_proc *proc,
struct list_head *list)
{
struct binder_work *w;
binder_inner_proc_lock(proc);
w = binder_dequeue_work_head_ilocked(list);
binder_inner_proc_unlock(proc);
return w;
}
static void static void
binder_defer_work(struct binder_proc *proc, enum binder_deferred_state defer); binder_defer_work(struct binder_proc *proc, enum binder_deferred_state defer);
static void binder_free_thread(struct binder_thread *thread); static void binder_free_thread(struct binder_thread *thread);
@@ -4753,13 +4732,17 @@ static void binder_release_work(struct binder_proc *proc,
struct list_head *list) struct list_head *list)
{ {
struct binder_work *w; struct binder_work *w;
enum binder_work_type wtype;
while (1) { while (1) {
w = binder_dequeue_work_head(proc, list); binder_inner_proc_lock(proc);
w = binder_dequeue_work_head_ilocked(list);
wtype = w ? w->type : 0;
binder_inner_proc_unlock(proc);
if (!w) if (!w)
return; return;
switch (w->type) { switch (wtype) {
case BINDER_WORK_TRANSACTION: { case BINDER_WORK_TRANSACTION: {
struct binder_transaction *t; struct binder_transaction *t;
@@ -4793,9 +4776,11 @@ static void binder_release_work(struct binder_proc *proc,
kfree(death); kfree(death);
binder_stats_deleted(BINDER_STAT_DEATH); binder_stats_deleted(BINDER_STAT_DEATH);
} break; } break;
case BINDER_WORK_NODE:
break;
default: default:
pr_err("unexpected work type, %d, not freed\n", pr_err("unexpected work type, %d, not freed\n",
w->type); wtype);
break; break;
} }
} }