From 77bfebf110773f5a0d6b5ff8110896adb2c9c335 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 5 Jun 2026 11:13:50 +0000 Subject: rust_binder: fix BINDER_GET_EXTENDED_ERROR This code currently copies the ExtendedError struct to the stack, modifies the copy, and then doesn't modify the original. Thus, fix it. Furthermore, errors when replying must be delivered directly to the remote thread, so update deliver_reply() to take an extended error argument. Cc: stable Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver") Signed-off-by: Alice Ryhl Acked-by: Carlos Llamas Link: https://patch.msgid.link/20260605-set-extended-error-v3-1-d60b69a75f97@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder/error.rs | 13 +++---- drivers/android/binder/thread.rs | 65 +++++++++++++++++++++++++---------- drivers/android/binder/transaction.rs | 15 ++++---- 3 files changed, 58 insertions(+), 35 deletions(-) (limited to 'drivers/android') diff --git a/drivers/android/binder/error.rs b/drivers/android/binder/error.rs index 45d85d4c2815..1296072c35d9 100644 --- a/drivers/android/binder/error.rs +++ b/drivers/android/binder/error.rs @@ -73,20 +73,17 @@ impl fmt::Debug for BinderError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.reply { BR_FAILED_REPLY => match self.source.as_ref() { - Some(source) => f - .debug_struct("BR_FAILED_REPLY") - .field("source", source) - .finish(), + Some(source) => source.fmt(f), None => f.pad("BR_FAILED_REPLY"), }, BR_DEAD_REPLY => f.pad("BR_DEAD_REPLY"), BR_FROZEN_REPLY => f.pad("BR_FROZEN_REPLY"), BR_TRANSACTION_PENDING_FROZEN => f.pad("BR_TRANSACTION_PENDING_FROZEN"), BR_TRANSACTION_COMPLETE => f.pad("BR_TRANSACTION_COMPLETE"), - _ => f - .debug_struct("BinderError") - .field("reply", &self.reply) - .finish(), + _ => match self.source.as_ref() { + Some(source) => source.fmt(f), + None => self.reply.fmt(f), + }, } } } diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs index 97d5f31e8fe3..3b8520813941 100644 --- a/drivers/android/binder/thread.rs +++ b/drivers/android/binder/thread.rs @@ -495,9 +495,16 @@ impl Thread { Ok(()) } + pub(crate) fn clear_extended_error(&self, debug_id: usize) { + self.inner.lock().extended_error = ExtendedError::new(debug_id as u32, BR_OK, 0); + } + pub(crate) fn get_extended_error(&self, data: UserSlice) -> Result { let mut writer = data.writer(); - let ee = self.inner.lock().extended_error; + let mut inner = self.inner.lock(); + let ee = inner.extended_error; + inner.extended_error = ExtendedError::new(0, BR_OK, 0); + drop(inner); writer.write(&ee)?; Ok(()) } @@ -1109,7 +1116,10 @@ impl Thread { inner.pop_transaction_to_reply(thread.as_ref()) } { let reply = Err(BR_DEAD_REPLY); - if !transaction.from.deliver_single_reply(reply, &transaction) { + if !transaction + .from + .deliver_single_reply(reply, &transaction, None) + { break; } @@ -1121,8 +1131,9 @@ impl Thread { &self, reply: Result, u32>, transaction: &DArc, + extended_error: Option, ) { - if self.deliver_single_reply(reply, transaction) { + if self.deliver_single_reply(reply, transaction, extended_error) { transaction.from.unwind_transaction_stack(); } } @@ -1136,6 +1147,7 @@ impl Thread { &self, reply: Result, u32>, transaction: &DArc, + extended_error: Option, ) -> bool { if let Ok(transaction) = &reply { crate::trace::trace_transaction(true, transaction, Some(&self.task)); @@ -1152,6 +1164,12 @@ impl Thread { return true; } + if let Some(ee) = extended_error { + if inner.extended_error.command == BR_OK { + inner.extended_error = ee; + } + } + match reply { Ok(work) => { inner.push_work(work); @@ -1222,6 +1240,9 @@ impl Thread { info.buffers_size = td.buffers_size as usize; // SAFETY: Above `read` call initializes all bytes, so this union read is ok. info.target_handle = unsafe { td.transaction_data.target.handle }; + + info.debug_id = super::next_debug_id(); + Ok(()) } @@ -1230,6 +1251,8 @@ impl Thread { let mut info = TransactionInfo::zeroed(); self.read_transaction_info(cmd, reader, &mut info)?; + self.clear_extended_error(info.debug_id); + let ret = if info.is_reply { self.reply_inner(&mut info) } else if info.is_oneway() { @@ -1239,23 +1262,21 @@ impl Thread { }; if let Err(err) = ret { - if err.reply != BR_TRANSACTION_COMPLETE { - info.reply = err.reply; - } - self.push_return_work(err.reply); - if let Some(source) = &err.source { - info.errno = source.to_errno(); + if err.reply != BR_TRANSACTION_COMPLETE { info.reply = err.reply; + if let Some(source) = &err.source { + info.errno = source.to_errno(); - { - let mut ee = self.inner.lock().extended_error; - ee.command = err.reply; - ee.param = source.to_errno(); + { + let mut inner = self.inner.lock(); + inner.extended_error = + ExtendedError::new(info.debug_id as u32, err.reply, source.to_errno()); + } } pr_warn!( - "{}:{} transaction to {} failed: {source:?}", + "{}:{} transaction to {} failed: {err:?}", info.from_pid, info.from_tid, info.to_pid @@ -1320,18 +1341,24 @@ impl Thread { let allow_fds = orig.flags & TF_ACCEPT_FDS != 0; let reply = Transaction::new_reply(self, process, info, allow_fds)?; self.inner.lock().push_work(completion); - orig.from.deliver_reply(Ok(reply), &orig); + orig.from.deliver_reply(Ok(reply), &orig, None); Ok(()) })() .map_err(|mut err| { // At this point we only return `BR_TRANSACTION_COMPLETE` to the caller, and we must let // the sender know that the transaction has completed (with an error in this case). + pr_warn!( - "Failure {:?} during reply - delivering BR_FAILED_REPLY to sender.", - err + "{}:{} reply to {} failed: {err:?}", + info.from_pid, + info.from_tid, + info.to_pid ); - let reply = Err(BR_FAILED_REPLY); - orig.from.deliver_reply(reply, &orig); + + let param = err.source.as_ref().map_or(0, |e| e.to_errno()); + let ee = ExtendedError::new(info.debug_id as u32, err.reply, param); + orig.from + .deliver_reply(Err(BR_FAILED_REPLY), &orig, Some(ee)); err.reply = BR_TRANSACTION_COMPLETE; err }); diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs index 1d9b66920a21..0e5d07b7e6f0 100644 --- a/drivers/android/binder/transaction.rs +++ b/drivers/android/binder/transaction.rs @@ -42,6 +42,7 @@ pub(crate) struct TransactionInfo { pub(crate) reply: u32, pub(crate) oneway_spam_suspect: bool, pub(crate) is_reply: bool, + pub(crate) debug_id: usize, } impl TransactionInfo { @@ -93,7 +94,6 @@ impl Transaction { from: &Arc, info: &mut TransactionInfo, ) -> BinderResult> { - let debug_id = super::next_debug_id(); let allow_fds = node_ref.node.flags & FLAT_BINDER_FLAG_ACCEPTS_FDS != 0; let txn_security_ctx = node_ref.node.flags & FLAT_BINDER_FLAG_TXN_SECURITY_CTX != 0; let mut txn_security_ctx_off = if txn_security_ctx { Some(0) } else { None }; @@ -101,7 +101,7 @@ impl Transaction { let mut alloc = match from.copy_transaction_data( to.clone(), info, - debug_id, + info.debug_id, allow_fds, txn_security_ctx_off.as_mut(), ) { @@ -128,7 +128,7 @@ impl Transaction { let data_address = alloc.ptr; Ok(DTRWrap::arc_pin_init(pin_init!(Transaction { - debug_id, + debug_id: info.debug_id, target_node: Some(target_node), from_parent, sender_euid: Kuid::current_euid(), @@ -152,9 +152,8 @@ impl Transaction { info: &mut TransactionInfo, allow_fds: bool, ) -> BinderResult> { - let debug_id = super::next_debug_id(); let mut alloc = - match from.copy_transaction_data(to.clone(), info, debug_id, allow_fds, None) { + match from.copy_transaction_data(to.clone(), info, info.debug_id, allow_fds, None) { Ok(alloc) => alloc, Err(err) => { pr_warn!("Failure in copy_transaction_data: {:?}", err); @@ -165,7 +164,7 @@ impl Transaction { alloc.set_info_clear_on_drop(); } Ok(DTRWrap::arc_pin_init(pin_init!(Transaction { - debug_id, + debug_id: info.debug_id, target_node: None, from_parent: None, sender_euid: Kuid::current_euid(), @@ -394,7 +393,7 @@ impl DeliverToRead for Transaction { let send_failed_reply = ScopeGuard::new(|| { if self.target_node.is_some() && self.flags & TF_ONE_WAY == 0 { let reply = Err(BR_FAILED_REPLY); - self.from.deliver_reply(reply, &self); + self.from.deliver_reply(reply, &self, None); } self.drop_outstanding_txn(); }); @@ -478,7 +477,7 @@ impl DeliverToRead for Transaction { // If this is not a reply or oneway transaction, then send a dead reply. if self.target_node.is_some() && self.flags & TF_ONE_WAY == 0 { let reply = Err(BR_DEAD_REPLY); - self.from.deliver_reply(reply, &self); + self.from.deliver_reply(reply, &self, None); } self.drop_outstanding_txn(); -- cgit v1.2.3 From b34826e55aad3520ec813f1f367c11b24b29dc9f Mon Sep 17 00:00:00 2001 From: Chris Mason Date: Wed, 3 Jun 2026 10:44:54 -0700 Subject: binder: cache secctx size before release zeroes it binder_transaction() bounds the scatter-gather buffer area with sg_buf_end_offset and subtracts the aligned LSM context size because the secctx is written at the tail of that area. The subtraction reads lsmctx.len, but that field has already been cleared by the time the line runs: security_secid_to_secctx(secid, &lsmctx) /* lsmctx.len set */ lsmctx_aligned_size = ALIGN(lsmctx.len, sizeof(u64)) extra_buffers_size += lsmctx_aligned_size ... security_release_secctx(&lsmctx) /* memset zeroes len */ ... sg_buf_end_offset = sg_buf_offset + extra_buffers_size - ALIGN(lsmctx.len, sizeof(u64)) /* ALIGN(0,8) */ security_release_secctx() does memset(cp, 0, sizeof(*cp)), so lsmctx.len reads back as 0 and the subtraction contributes nothing, leaving sg_buf_end_offset too large by the aligned secctx size on every transaction to a txn_security_ctx node. Each BINDER_TYPE_PTR object then derives buf_left = sg_buf_end_offset - sg_buf_offset as the sole upper bound on its copy, so the inflated end offset lets the copy run into the bytes that already hold the secctx. The aligned size must therefore be cached before release rather than re-read from the now-cleared field. Fix by caching it in lsmctx_aligned_size at function scope when it is first computed and subtracting lsmctx_aligned_size instead of re-reading lsmctx.len after release. Reuse the same value for the earlier buf_offset computation. Fixes: 6fba89813ccf ("lsm: ensure the correct LSM context releaser") Cc: stable Assisted-by: kres:claude-opus-4-8 Signed-off-by: Chris Mason Reviewed-by: Alice Ryhl Acked-by: Carlos Llamas Link: https://patch.msgid.link/20260603174506.1957278-1-clm@meta.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/android') diff --git a/drivers/android/binder.c b/drivers/android/binder.c index ec0ab4f28530..c48c22264266 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -3080,6 +3080,7 @@ static void binder_transaction(struct binder_proc *proc, int t_debug_id = atomic_inc_return(&binder_last_id); ktime_t t_start_time = ktime_get(); struct lsm_context lsmctx = { }; + size_t lsmctx_aligned_size = 0; LIST_HEAD(sgc_head); LIST_HEAD(pf_head); const void __user *user_buffer = (const void __user *) @@ -3346,7 +3347,6 @@ static void binder_transaction(struct binder_proc *proc, if (target_node && target_node->txn_security_ctx) { u32 secid; - size_t added_size; security_cred_getsecid(proc->cred, &secid); ret = security_secid_to_secctx(secid, &lsmctx); @@ -3358,9 +3358,9 @@ static void binder_transaction(struct binder_proc *proc, return_error_line = __LINE__; goto err_get_secctx_failed; } - added_size = ALIGN(lsmctx.len, sizeof(u64)); - extra_buffers_size += added_size; - if (extra_buffers_size < added_size) { + lsmctx_aligned_size = ALIGN(lsmctx.len, sizeof(u64)); + extra_buffers_size += lsmctx_aligned_size; + if (extra_buffers_size < lsmctx_aligned_size) { binder_txn_error("%d:%d integer overflow of extra_buffers_size\n", thread->pid, proc->pid); return_error = BR_FAILED_REPLY; @@ -3397,7 +3397,7 @@ static void binder_transaction(struct binder_proc *proc, size_t buf_offset = ALIGN(tr->data_size, sizeof(void *)) + ALIGN(tr->offsets_size, sizeof(void *)) + ALIGN(extra_buffers_size, sizeof(void *)) - - ALIGN(lsmctx.len, sizeof(u64)); + lsmctx_aligned_size; t->security_ctx = t->buffer->user_data + buf_offset; err = binder_alloc_copy_to_buffer(&target_proc->alloc, @@ -3452,7 +3452,7 @@ static void binder_transaction(struct binder_proc *proc, off_end_offset = off_start_offset + tr->offsets_size; sg_buf_offset = ALIGN(off_end_offset, sizeof(void *)); sg_buf_end_offset = sg_buf_offset + extra_buffers_size - - ALIGN(lsmctx.len, sizeof(u64)); + lsmctx_aligned_size; off_min = 0; for (buffer_offset = off_start_offset; buffer_offset < off_end_offset; buffer_offset += sizeof(binder_size_t)) { -- cgit v1.2.3 From eb1645bf10190e71f6f0316e37ff70755d719b53 Mon Sep 17 00:00:00 2001 From: Keshav Verma Date: Tue, 16 Jun 2026 02:47:43 +0530 Subject: rust_binder: synchronize Rust Binder stats with freeze commands Rust Binder stats use BC_COUNT and BR_COUNT to size the command and return counters, and use event string tables when printing debug statistics. The Binder protocol includes freeze-related commands and return codes, but the Rust Binder statistics code was not updated to cover them. As a result, those commands and return codes are not accounted for or printed by the stats debug output. Update the counts and event string tables so these commands and return codes are included in the debug statistics output. Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver") Cc: stable Acked-by: Carlos Llamas Reviewed-by: Alice Ryhl Signed-off-by: Keshav Verma Link: https://patch.msgid.link/20260615211743.734-1-iganschel@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder/rust_binder_events.c | 7 ++++++- drivers/android/binder/stats.rs | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) (limited to 'drivers/android') diff --git a/drivers/android/binder/rust_binder_events.c b/drivers/android/binder/rust_binder_events.c index 488b1470060c..5792aa59cc82 100644 --- a/drivers/android/binder/rust_binder_events.c +++ b/drivers/android/binder/rust_binder_events.c @@ -28,6 +28,9 @@ const char * const binder_command_strings[] = { "BC_DEAD_BINDER_DONE", "BC_TRANSACTION_SG", "BC_REPLY_SG", + "BC_REQUEST_FREEZE_NOTIFICATION", + "BC_CLEAR_FREEZE_NOTIFICATION", + "BC_FREEZE_NOTIFICATION_DONE", }; const char * const binder_return_strings[] = { @@ -51,7 +54,9 @@ const char * const binder_return_strings[] = { "BR_FAILED_REPLY", "BR_FROZEN_REPLY", "BR_ONEWAY_SPAM_SUSPECT", - "BR_TRANSACTION_PENDING_FROZEN" + "BR_TRANSACTION_PENDING_FROZEN", + "BR_FROZEN_BINDER", + "BR_CLEAR_FREEZE_NOTIFICATION_DONE", }; #define CREATE_TRACE_POINTS diff --git a/drivers/android/binder/stats.rs b/drivers/android/binder/stats.rs index ab75e9561cbf..ec81dc7747db 100644 --- a/drivers/android/binder/stats.rs +++ b/drivers/android/binder/stats.rs @@ -8,8 +8,8 @@ use crate::defs::*; use kernel::sync::atomic::{ordering::Relaxed, Atomic}; use kernel::{ioctl::_IOC_NR, seq_file::SeqFile, seq_print}; -const BC_COUNT: usize = _IOC_NR(BC_REPLY_SG) as usize + 1; -const BR_COUNT: usize = _IOC_NR(BR_TRANSACTION_PENDING_FROZEN) as usize + 1; +const BC_COUNT: usize = _IOC_NR(BC_FREEZE_NOTIFICATION_DONE) as usize + 1; +const BR_COUNT: usize = _IOC_NR(BR_CLEAR_FREEZE_NOTIFICATION_DONE) as usize + 1; pub(crate) static GLOBAL_STATS: BinderStats = BinderStats::new(); -- cgit v1.2.3 From 114a116aaa5f0295376cdf12da743c5bce3b20ce Mon Sep 17 00:00:00 2001 From: Carlos Llamas Date: Fri, 19 Jun 2026 18:52:30 +0000 Subject: binder: fix UAF in binder_thread_release() 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 Fixes: 7a4408c6bd3e ("binder: make sure accesses to proc/thread are safe") Reviewed-by: Alice Ryhl Signed-off-by: Carlos Llamas Link: https://patch.msgid.link/20260619185233.2194678-1-cmllamas@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/android') diff --git a/drivers/android/binder.c b/drivers/android/binder.c index c48c22264266..013e2bfab070 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); -- cgit v1.2.3 From f223d27a546c1e1f48d38fd67760e78f068fe8c4 Mon Sep 17 00:00:00 2001 From: Carlos Llamas Date: Fri, 19 Jun 2026 18:52:31 +0000 Subject: 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 Reported-by: Alice Ryhl Closes: https://lore.kernel.org/all/aikJKVuny_eOivwN@google.com/ Fixes: a370003cc301 ("binder: fix possible UAF when freeing buffer") Signed-off-by: Carlos Llamas Reviewed-by: Alice Ryhl Link: https://patch.msgid.link/20260619185233.2194678-2-cmllamas@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'drivers/android') diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 013e2bfab070..8f2ef1bd539f 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -1658,10 +1658,19 @@ static void binder_txn_latency_free(struct binder_transaction *t) static void binder_free_transaction(struct binder_transaction *t) { + struct binder_thread *target_thread; struct binder_proc *target_proc; spin_lock(&t->lock); target_proc = t->to_proc; + target_thread = t->to_thread; + /* + * Pin target_thread to keep target_proc alive. Undelivered + * transactions with !target_thread are safe, as target_proc + * can only be the current context there. + */ + if (target_thread) + atomic_inc(&target_thread->tmp_ref); spin_unlock(&t->lock); if (target_proc) { @@ -1676,6 +1685,10 @@ static void binder_free_transaction(struct binder_transaction *t) t->buffer->transaction = NULL; binder_inner_proc_unlock(target_proc); } + + if (target_thread) + binder_thread_dec_tmpref(target_thread); + if (trace_binder_txn_latency_free_enabled()) binder_txn_latency_free(t); /* -- cgit v1.2.3 From 803c8a9502e9b97cd6ae937618ef4a8fd6274343 Mon Sep 17 00:00:00 2001 From: Hyunwoo Kim Date: Sun, 31 May 2026 22:29:24 +0900 Subject: rust_binder: use a u64 stride when cleaning up the offsets array Allocation's Drop walks the offsets array (binder_size_t = u64 entries), cleaning up the objects, but it used usize instead of u64 for both the stride and the per-entry read. On 64-bit kernels (usize == u64) this is harmless, but on 32-bit kernels it walks the 8-byte entries in 4-byte steps, iterating an N-entry array 2N times, and reads the always-zero high word as offset 0, cleaning up the object at offset 0 N extra times. As a result the referenced node or handle ends up with a lower reference count than it actually has (a refcount over-decrement), and binder's reference accounting is corrupted; for example, the owner can be notified of a strong reference release (BR_RELEASE) even though references still remain. Change the stride to u64, and read each entry as a u64, narrowing it to usize with try_into(). On 32-bit ARM, when this over-decrement would drive a count below zero, the driver's existing refcount guard refuses it and fires: rust_binder: Failure: refcount underflow! Cc: stable Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver") Signed-off-by: Hyunwoo Kim Acked-by: Carlos Llamas Reviewed-by: Alice Ryhl Link: https://patch.msgid.link/ahw3tFhLz9bMMJAO@v4bel Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder/allocation.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/android') diff --git a/drivers/android/binder/allocation.rs b/drivers/android/binder/allocation.rs index b7b05e72970a..ea5846e4da16 100644 --- a/drivers/android/binder/allocation.rs +++ b/drivers/android/binder/allocation.rs @@ -259,7 +259,7 @@ impl Drop for Allocation { if let Some(offsets) = info.offsets.clone() { let view = AllocationView::new(self, offsets.start); - for i in offsets.step_by(size_of::()) { + for i in offsets.step_by(size_of::()) { if view.cleanup_object(i).is_err() { pr_warn!("Error cleaning up object at offset {}\n", i) } @@ -420,7 +420,8 @@ impl<'a> AllocationView<'a> { } fn cleanup_object(&self, index_offset: usize) -> Result { - let offset = self.alloc.read(index_offset)?; + let offset = self.alloc.read::(index_offset)?; + let offset: usize = offset.try_into().map_err(|_| EINVAL)?; let header = self.read::(offset)?; match header.type_ { BINDER_TYPE_WEAK_BINDER | BINDER_TYPE_BINDER => { -- cgit v1.2.3 From 6849cabfd30fb5727cfd31e8241e15801e17ebf9 Mon Sep 17 00:00:00 2001 From: Keshav Verma Date: Thu, 25 Jun 2026 16:09:57 +0530 Subject: rust_binder: reject context manager self-transaction Rust binder resolved handle 0 to the context manager node, but it does not reject the case where the caller owns the same node. The C binder driver rejects transactions from the context-manager process to handle 0 after resolving the target node. Match that behavior in Rust Binder by rejecting handle 0 transactions when the resolved context-manager node is owned by the calling process. This applies to both synchronous and oneway transactions because both paths resolve the target through Process::get_transaction_node(). Cc: stable Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver") Signed-off-by: Keshav Verma Reviewed-by: Alice Ryhl Link: https://patch.msgid.link/20260625103957.730-1-iganschel@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder/process.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/android') diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs index 96b8440ceac6..ca664fda8e81 100644 --- a/drivers/android/binder/process.rs +++ b/drivers/android/binder/process.rs @@ -900,7 +900,11 @@ impl Process { pub(crate) fn get_transaction_node(&self, handle: u32) -> BinderResult { // When handle is zero, try to get the context manager. if handle == 0 { - Ok(self.ctx.get_manager_node(true)?) + let node_ref = self.ctx.get_manager_node(true)?; + if core::ptr::eq(self, &*node_ref.node.owner) { + return Err(EINVAL.into()); + } + Ok(node_ref) } else { Ok(self.get_node_from_handle(handle, true)?) } -- cgit v1.2.3 From bc4a9828897871ff3e5a1f8a1d346decbf4ee95e Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 3 Jul 2026 11:25:12 +0000 Subject: rust_binder: clear freeze listener on node removal Generally userspace is supposed to explicitly clear freeze listeners before they drop the refcount on the node ref to zero, but there's nothing forcing that. Currently, in this scenario the freeze listener remains in the freeze_listeners rbtree and in the remote node's freeze listener list, even though the ref for which the listener is registered is gone. This could potentially lead to a memory leak due to a refcount cycle. Thus, remove the freeze listener in this scenario. Cc: stable Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver") Signed-off-by: Alice Ryhl Link: https://patch.msgid.link/20260703-remove-freeze-on-remove-node-v3-1-6e0c4547af46@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder/freeze.rs | 11 +++++++++-- drivers/android/binder/node.rs | 10 ++++++---- drivers/android/binder/process.rs | 12 +++++++++++- 3 files changed, 26 insertions(+), 7 deletions(-) (limited to 'drivers/android') diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs index 53b60035639a..f4df14568b25 100644 --- a/drivers/android/binder/freeze.rs +++ b/drivers/android/binder/freeze.rs @@ -154,10 +154,17 @@ impl DeliverToRead for FreezeMessage { } impl FreezeListener { - pub(crate) fn on_process_exit(&self, proc: &Arc) { + /// Called when this freeze listener is cleared abnormally. + /// + /// This occurs either because the process exited or because the process dropped its last + /// refcount on the node ref without explicitly removing the freeze listener first. + /// + /// The returned `KVVec` is just a value that should be dropped outside of the lock. + pub(crate) fn on_process_cleanup(&self, proc: &Process) -> KVVec> { if !self.is_clearing { - self.node.remove_freeze_listener(proc); + return self.node.remove_freeze_listener(proc); } + KVVec::new() } } diff --git a/drivers/android/binder/node.rs b/drivers/android/binder/node.rs index 69f757ff7461..c10148e9069f 100644 --- a/drivers/android/binder/node.rs +++ b/drivers/android/binder/node.rs @@ -682,12 +682,13 @@ impl Node { } } - pub(crate) fn remove_freeze_listener(&self, p: &Arc) { - let _unused_capacity; + pub(crate) fn remove_freeze_listener(&self, p: &Process) -> KVVec> { let mut guard = self.owner.inner.lock(); let inner = self.inner.access_mut(&mut guard); let len = inner.freeze_list.len(); - inner.freeze_list.retain(|proc| !Arc::ptr_eq(proc, p)); + inner + .freeze_list + .retain(|proc| !core::ptr::eq::(&**proc, p)); if len == inner.freeze_list.len() { pr_warn!( "Could not remove freeze listener for {}\n", @@ -695,8 +696,9 @@ impl Node { ); } if inner.freeze_list.is_empty() { - _unused_capacity = mem::take(&mut inner.freeze_list); + return mem::take(&mut inner.freeze_list); } + KVVec::new() } pub(crate) fn freeze_list<'a>(&'a self, guard: &'a ProcessInner) -> &'a [Arc] { diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs index ca664fda8e81..cdd1a9079726 100644 --- a/drivers/android/binder/process.rs +++ b/drivers/android/binder/process.rs @@ -946,6 +946,8 @@ impl Process { // To preserve original binder behaviour, we only fail requests where the manager tries to // increment references on itself. + let _to_free_freeze_listener; + let _to_free_freeze_listener_cleanup; let mut refs = self.node_refs.lock(); if let Some(info) = refs.by_handle.get_mut(&handle) { if info.node_ref().update(inc, strong) { @@ -961,6 +963,14 @@ impl Process { unsafe { info.node_ref2().node.remove_node_info(info) }; let id = info.node_ref().node.global_id(); + + if let Some(freeze) = *info.freeze() { + if let Some(fl) = refs.freeze_listeners.remove(&freeze) { + _to_free_freeze_listener_cleanup = fl.on_process_cleanup(&self); + _to_free_freeze_listener = fl; + } + } + refs.by_handle.remove(&handle); refs.by_node.remove(&id); refs.handle_is_present.release_id(handle as usize); @@ -1384,7 +1394,7 @@ impl Process { // Clean up freeze listeners. let freeze_listeners = take(&mut self.node_refs.lock().freeze_listeners); for listener in freeze_listeners.values() { - listener.on_process_exit(&self); + listener.on_process_cleanup(&self); } drop(freeze_listeners); -- cgit v1.2.3 From bb66b1a3452534adb8b72abf2f761375970fe472 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Wed, 8 Jul 2026 12:00:26 +0000 Subject: rust_binder: only print failure if error has source The commit that fixes BINDER_GET_EXTENDED_ERROR changed the condition for printing transaction failures so errors are printed even if the cause is a dead or frozen process. Undo this change so that the error is only printed if the failure has an errno associated with it. Cc: stable@kernel.org Fixes: 77bfebf11077 ("rust_binder: fix BINDER_GET_EXTENDED_ERROR") Signed-off-by: Alice Ryhl Link: https://patch.msgid.link/20260708-get-extended-error-fix-printing-v1-1-6e293b213b70@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder/thread.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers/android') diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs index 3b8520813941..bc0ef8927905 100644 --- a/drivers/android/binder/thread.rs +++ b/drivers/android/binder/thread.rs @@ -1273,14 +1273,14 @@ impl Thread { inner.extended_error = ExtendedError::new(info.debug_id as u32, err.reply, source.to_errno()); } - } - pr_warn!( - "{}:{} transaction to {} failed: {err:?}", - info.from_pid, - info.from_tid, - info.to_pid - ); + pr_warn!( + "{}:{} transaction to {} failed: {err:?}", + info.from_pid, + info.from_tid, + info.to_pid + ); + } } } -- cgit v1.2.3 From dd7aea9ee2091cfae3a5e376af87aa106d7735cd Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Mon, 27 Jul 2026 12:28:59 +0000 Subject: rust_binder: do not query current thread for all ioctls The get_current_thread() method is currently called for every ioctl to ensure that a Thread struct exists for the thread calling into the driver. However, not all ioctls require a Thread object, so this means we are unnecessarily creating these objects in cases where we don't need to. If said thread does not invoke BINDER_THREAD_EXIT on exit, Binder's Thread struct stays around until the fd is closed. For long-lived processes the Thread object is effectively leaked. Furthermore, when the BINDER_GET_NODE_DEBUG_INFO ioctl is invoked by libmemunreachable to ensure that objects reachable only through the Binder driver are not considered leaked, this is done from a fork of the process owning the fd, which means that it fails the group_leader check inside get_current_thread(). This results in EINVAL errors for this ioctl, causing libmemunreachable to report a false positive memory leak. Thus, do not invoke get_current_thread() for ioctls that do not require it. Signed-off-by: Alice Ryhl Cc: stable Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver") Acked-by: Carlos Llamas Link: https://patch.msgid.link/20260727-binder-cur-thread-v1-1-8edf2b64e235@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder/process.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'drivers/android') diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs index cdd1a9079726..5b8f73ec1931 100644 --- a/drivers/android/binder/process.rs +++ b/drivers/android/binder/process.rs @@ -1586,6 +1586,10 @@ impl Process { cmd: u32, reader: &mut UserSliceReader, ) -> Result { + if cmd == uapi::BINDER_FREEZE { + return ioctl_freeze(reader); + } + let thread = this.get_current_thread()?; match cmd { uapi::BINDER_SET_MAX_THREADS => this.set_max_threads(reader.read()?), @@ -1597,7 +1601,6 @@ impl Process { uapi::BINDER_ENABLE_ONEWAY_SPAM_DETECTION => { this.set_oneway_spam_detection_enabled(reader.read()?) } - uapi::BINDER_FREEZE => ioctl_freeze(reader)?, _ => return Err(EINVAL), } Ok(()) @@ -1612,15 +1615,16 @@ impl Process { cmd: u32, data: UserSlice, ) -> Result { - let thread = this.get_current_thread()?; let blocking = (file.flags() & file::flags::O_NONBLOCK) == 0; match cmd { - uapi::BINDER_WRITE_READ => thread.write_read(data, blocking)?, + uapi::BINDER_WRITE_READ => this.get_current_thread()?.write_read(data, blocking)?, uapi::BINDER_GET_NODE_DEBUG_INFO => this.get_node_debug_info(data)?, uapi::BINDER_GET_NODE_INFO_FOR_REF => this.get_node_info_from_ref(data)?, uapi::BINDER_VERSION => this.version(data)?, uapi::BINDER_GET_FROZEN_INFO => get_frozen_status(data)?, - uapi::BINDER_GET_EXTENDED_ERROR => thread.get_extended_error(data)?, + uapi::BINDER_GET_EXTENDED_ERROR => { + this.get_current_thread()?.get_extended_error(data)? + } _ => return Err(EINVAL), } Ok(()) -- cgit v1.2.3