Evolution-X-Devices/kernel_google_wahoo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ANDROID: binder: fix race in thread cleanup.

Browse Source 
Let's say we have the following transaction
stack:

Thread1 - t1 -> Thread2
Thread2 - t2 -> Thread1
Thread1 - t3 -> Thread3

The processes hosting Thread1 and Thread 2 both die;
when Thread3 tries to send a reply to t3, it finds
that the process is dead, and calls
binder_send_failed_reply() on t3. At the same time,
binder_free_thread() starts running on Thread1
and Thread2.

binder_free_thread() walks the transaction stack of
Thread2 first, and makes the following change:

t2->from = NULL;

binder_free_thread() then walks the transaction
stack of Thread1, and makes the following change:

t3->from = NULL;

It then suspends, and binder_send_failed_reply(t3)
runs; it sees t3->from = NULL, so continues walking
the transaction stack. It finds t2->from = NULL,
and continues with t1. It sees that t1->from is
still set, and assigns it to target_thread. At
that point, it tries to take the proc lock of that
thread, and blocks because binder_free_thread()
still has that lock. binder_free_thread() completes,
and drops the lock.

binder_send_failed_reply() acquires the lock,
and tries to pop t1 from the transaction stack.
This fails with a BUG_ON(), because the transaction
stack of Thread1 is still pointing to t3.

Fixed by checking whether we've marked the thread
as a zombie after taking the lock; in that case,
just continue unwinding the stack (if needed).

Bug: 64074090
Change-Id: I3b5ca0058b14b21038764147f78186310b812f80
Signed-off-by: Martijn Coenen <maco@android.com>
This commit is contained in:
Martijn Coenen
2017-08-02 10:11:56 +02:00
committed by Martijn Coenen
parent f122f32705
commit 991e8349d2
1 changed files with 13 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
drivers/android/binder.c
View File

@@ -1379,7 +1379,19 @@ static void binder_send_failed_reply(struct binder_transaction *t,
target_thread = t->from;
if (target_thread) {
binder_proc_lock(target_thread->proc, __LINE__);
/*
* It's possible that binder_free_thread() just marked
* the thread a zombie before we took the lock; in that
* case, there's no point in queueing any work to the
* thread - proceed as if it's already dead.
*/
if (target_thread->is_zombie) {
binder_proc_unlock(target_thread->proc,
__LINE__);
target_thread = NULL;
}
}
if (target_thread) {
binder_debug(BINDER_DEBUG_FAILED_TRANSACTION,
"send failed reply for transaction %d to %d:%d\n",
t->debug_id,