* [PATCH] binder: fix UAF in binder_thread_release()
@ 2026-06-06 2:22 Carlos Llamas
2026-06-10 6:50 ` Alice Ryhl
0 siblings, 1 reply; 2+ messages in thread
From: Carlos Llamas @ 2026-06-06 2:22 UTC (permalink / raw)
To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Christian Brauner, Carlos Llamas, Alice Ryhl
Cc: kernel-team, linux-kernel, stable
When a thread exits, binder_thread_release() walks its transaction stack
to clear the t->from and t->to_proc that correspond with the exiting
thread. However, a process dying in parallel might attempt to kfree some
of these transactions. And if one of them has no associated t->to_proc,
the t->to_proc->inner_lock will not be acquired.
This means that transaction accesses in binder_thread_release() after
t->to_proc has been cleared might race with binder_free_transaction()
and cause a use-after-free error as reported by KASAN:
==================================================================
BUG: KASAN: slab-use-after-free in binder_thread_release+0x5d0/0x798
Write of size 8 at addr ffff000016627500 by task X/715
CPU: 17 UID: 0 PID: 715 Comm: X Not tainted 7.1.0-rc5-00149-g8fde5d1d47f6 #30 PREEMPT
Hardware name: linux,dummy-virt (DT)
Call trace:
binder_thread_release+0x5d0/0x798
binder_ioctl+0x12c0/0x299c
[...]
Allocated by task 717 on cpu 18 at 67.267803s:
__kasan_kmalloc+0xa0/0xbc
__kmalloc_cache_noprof+0x174/0x444
binder_transaction+0x554/0x8150
binder_thread_write+0xa30/0x4354
binder_ioctl+0x20f0/0x299c
[...]
Freed by task 202 on cpu 18 at 90.416221s:
__kasan_slab_free+0x58/0x80
kfree+0x1a0/0x4a4
binder_free_transaction+0x150/0x294
binder_send_failed_reply+0x398/0x6d8
binder_release_work+0x3e4/0x4ec
binder_deferred_func+0xbd8/0x104c
[...]
==================================================================
In order to avoid this, make sure that binder_free_transaction() reads
the t->to_proc under the transaction lock. This will serialize the
transaction release with the accesses in binder_thread_release(). Plus,
it matches the documented locking rules for @to_proc.
Cc: stable@vger.kernel.org
Fixes: 7a4408c6bd3e ("binder: make sure accesses to proc/thread are safe")
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
drivers/android/binder.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9e6194224593..09bc052186cf 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -1658,7 +1658,11 @@ static void binder_txn_latency_free(struct binder_transaction *t)
static void binder_free_transaction(struct binder_transaction *t)
{
- struct binder_proc *target_proc = t->to_proc;
+ struct binder_proc *target_proc;
+
+ spin_lock(&t->lock);
+ target_proc = t->to_proc;
+ spin_unlock(&t->lock);
if (target_proc) {
binder_inner_proc_lock(target_proc);
--
2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] binder: fix UAF in binder_thread_release()
2026-06-06 2:22 [PATCH] binder: fix UAF in binder_thread_release() Carlos Llamas
@ 2026-06-10 6:50 ` Alice Ryhl
0 siblings, 0 replies; 2+ messages in thread
From: Alice Ryhl @ 2026-06-10 6:50 UTC (permalink / raw)
To: Carlos Llamas
Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Christian Brauner, kernel-team, linux-kernel, stable
On Sat, Jun 06, 2026 at 02:22:32AM +0000, Carlos Llamas wrote:
> When a thread exits, binder_thread_release() walks its transaction stack
> to clear the t->from and t->to_proc that correspond with the exiting
> thread. However, a process dying in parallel might attempt to kfree some
> of these transactions. And if one of them has no associated t->to_proc,
> the t->to_proc->inner_lock will not be acquired.
>
> This means that transaction accesses in binder_thread_release() after
> t->to_proc has been cleared might race with binder_free_transaction()
> and cause a use-after-free error as reported by KASAN:
>
> ==================================================================
> BUG: KASAN: slab-use-after-free in binder_thread_release+0x5d0/0x798
> Write of size 8 at addr ffff000016627500 by task X/715
>
> CPU: 17 UID: 0 PID: 715 Comm: X Not tainted 7.1.0-rc5-00149-g8fde5d1d47f6 #30 PREEMPT
> Hardware name: linux,dummy-virt (DT)
> Call trace:
> binder_thread_release+0x5d0/0x798
> binder_ioctl+0x12c0/0x299c
> [...]
>
> Allocated by task 717 on cpu 18 at 67.267803s:
> __kasan_kmalloc+0xa0/0xbc
> __kmalloc_cache_noprof+0x174/0x444
> binder_transaction+0x554/0x8150
> binder_thread_write+0xa30/0x4354
> binder_ioctl+0x20f0/0x299c
> [...]
>
> Freed by task 202 on cpu 18 at 90.416221s:
> __kasan_slab_free+0x58/0x80
> kfree+0x1a0/0x4a4
> binder_free_transaction+0x150/0x294
> binder_send_failed_reply+0x398/0x6d8
> binder_release_work+0x3e4/0x4ec
> binder_deferred_func+0xbd8/0x104c
> [...]
> ==================================================================
>
> In order to avoid this, make sure that binder_free_transaction() reads
> the t->to_proc under the transaction lock. This will serialize the
> transaction release with the accesses in binder_thread_release(). Plus,
> it matches the documented locking rules for @to_proc.
>
> Cc: stable@vger.kernel.org
> Fixes: 7a4408c6bd3e ("binder: make sure accesses to proc/thread are safe")
> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> ---
> drivers/android/binder.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 9e6194224593..09bc052186cf 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -1658,7 +1658,11 @@ static void binder_txn_latency_free(struct binder_transaction *t)
>
> static void binder_free_transaction(struct binder_transaction *t)
> {
> - struct binder_proc *target_proc = t->to_proc;
> + struct binder_proc *target_proc;
> +
> + spin_lock(&t->lock);
> + target_proc = t->to_proc;
> + spin_unlock(&t->lock);
Although I don't think this fixes all issues here, as we discussed more
in private, this does fix the specific UAF referenced in this patch, so:
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
The logic is that either binder_free_transaction() reads a non-null
target_proc, in which case we take the inner proc lock and fully
synchronize with the entirety of binder_thread_release(), or we read a
null target_proc in which case the transaction spinlock ensures that we
wait for the part of binder_thread_release() touching this particular
'struct binder_transaction'.
Alice
> if (target_proc) {
> binder_inner_proc_lock(target_proc);
> --
> 2.54.0.1032.g2f8565e1d1-goog
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-06-10 6:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-06 2:22 [PATCH] binder: fix UAF in binder_thread_release() Carlos Llamas
2026-06-10 6:50 ` Alice Ryhl
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.