Skip to content

Commit f223d27

Browse files
Carlos Llamasgregkh
authored andcommitted
binder: fix UAF in binder_free_transaction()
In binder_free_transaction(), the t->to_proc is read under the t->lock. However, once the t->lock is dropped, the to_proc can die in parallel. This leads to a use-after-free error when we attempt to acquire its inner lock right afterwards: ================================================================== BUG: KASAN: slab-use-after-free in _raw_spin_lock+0xe4/0x1a0 Write of size 4 at addr ffff00001125da70 by task B/672 CPU: 20 UID: 0 PID: 672 Comm: B Not tainted 7.1.0-rc6-00284-g8e65320d91cd #4 PREEMPT Hardware name: linux,dummy-virt (DT) Call trace: _raw_spin_lock+0xe4/0x1a0 binder_free_transaction+0x8c/0x320 binder_send_failed_reply+0x21c/0x2f8 binder_thread_release+0x488/0x7e0 binder_ioctl+0x12c0/0x29a0 [...] Allocated by task 675: __kmalloc_cache_noprof+0x174/0x444 binder_open+0x118/0xb70 do_dentry_open+0x374/0x1040 vfs_open+0x58/0x3bc [...] Freed by task 212: __kasan_slab_free+0x58/0x80 kfree+0x1a0/0x4a4 binder_proc_dec_tmpref+0x32c/0x5e0 binder_deferred_func+0xc48/0x104c process_one_work+0x53c/0xbc0 [...] ================================================================== To prevent this, pin the target thread (t->to_thread) to guarantee the target process remains alive. Undelivered transactions without a target thread are already safe, as the target process can only be the current context in those paths. Cc: stable <stable@kernel.org> Reported-by: Alice Ryhl <aliceryhl@google.com> Closes: https://lore.kernel.org/all/aikJKVuny_eOivwN@google.com/ Fixes: a370003 ("binder: fix possible UAF when freeing buffer") Signed-off-by: Carlos Llamas <cmllamas@google.com> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://patch.msgid.link/20260619185233.2194678-2-cmllamas@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 114a116 commit f223d27

1 file changed

Lines changed: 13 additions & 0 deletions

File tree

drivers/android/binder.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1658,10 +1658,19 @@ static void binder_txn_latency_free(struct binder_transaction *t)
16581658

16591659
static void binder_free_transaction(struct binder_transaction *t)
16601660
{
1661+
struct binder_thread *target_thread;
16611662
struct binder_proc *target_proc;
16621663

16631664
spin_lock(&t->lock);
16641665
target_proc = t->to_proc;
1666+
target_thread = t->to_thread;
1667+
/*
1668+
* Pin target_thread to keep target_proc alive. Undelivered
1669+
* transactions with !target_thread are safe, as target_proc
1670+
* can only be the current context there.
1671+
*/
1672+
if (target_thread)
1673+
atomic_inc(&target_thread->tmp_ref);
16651674
spin_unlock(&t->lock);
16661675

16671676
if (target_proc) {
@@ -1676,6 +1685,10 @@ static void binder_free_transaction(struct binder_transaction *t)
16761685
t->buffer->transaction = NULL;
16771686
binder_inner_proc_unlock(target_proc);
16781687
}
1688+
1689+
if (target_thread)
1690+
binder_thread_dec_tmpref(target_thread);
1691+
16791692
if (trace_binder_txn_latency_free_enabled())
16801693
binder_txn_latency_free(t);
16811694
/*

0 commit comments

Comments
 (0)