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

Commit 0cbcd22d authored by Todd Kjos's avatar Todd Kjos Committed by shaohanlin
Browse files

binder: fix UAF when releasing todo list 



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() calls 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.

Bug: 161151868
Signed-off-by: default avatarTodd Kjos <tkjos@google.com>
Change-Id: I4ae752abfe1aa38872be6f266ddd271802952625
parent e34be057

drivers/android/binder.c

+10 −25
Original line number Diff line number Diff line
@@ -281,7 +281,7 @@ struct binder_device { 
struct binder_work {

	struct list_head entry;



	enum {

	enum binder_work_type {

		BINDER_WORK_TRANSACTION = 1,

		BINDER_WORK_TRANSACTION_COMPLETE,

		BINDER_WORK_RETURN_ERROR,
@@ -938,27 +938,6 @@ static struct binder_work *binder_dequeue_work_head_ilocked( 
	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

binder_defer_work(struct binder_proc *proc, enum binder_deferred_state defer);

static void binder_free_thread(struct binder_thread *thread);
@@ -4584,13 +4563,17 @@ static void binder_release_work(struct binder_proc *proc, 
				struct list_head *list)

{

	struct binder_work *w;

	enum binder_work_type wtype;



	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)

			return;



		switch (w->type) {

		switch (wtype) {

		case BINDER_WORK_TRANSACTION: {

			struct binder_transaction *t;
@@ -4624,9 +4607,11 @@ static void binder_release_work(struct binder_proc *proc, 
			kfree(death);

			binder_stats_deleted(BINDER_STAT_DEATH);

		} break;

		case BINDER_WORK_NODE:

			break;

		default:

			pr_err("unexpected work type, %d, not freed\n",

			       w->type);

			       wtype);

			break;

		}

	}