* [PATCH 1/2] rust_binder: reject sync transaction with pending sync work
@ 2026-11-08 15:11 Chris Wiseman
2026-11-08 15:11 ` [PATCH 2/2] rust_binder: account for deferred work size in Thread::read() Chris Wiseman
2026-11-09 19:15 ` [PATCH 1/2] rust_binder: reject sync transaction with pending sync work Carlos Llamas
0 siblings, 2 replies; 4+ messages in thread
From: Chris Wiseman @ 2026-11-08 15:11 UTC (permalink / raw)
To: Alice Ryhl, Carlos Llamas, Greg Kroah-Hartman
Cc: Arve Hjønnevåg, Todd Kjos, Christian Brauner,
Miguel Ojeda, rust-for-linux, linux-kernel, stable,
Chris Wiseman
When a thread replies to a transaction with TF_DEFER_COMPLETE, its
BR_TRANSACTION_COMPLETE work item stays in inner.work_list while the
next blocking read waits in Process.ready_threads. A new incoming
synchronous transaction can then be queued behind that deferred
completion in inner.work_list. If the thread's read buffer only has
room for the deferred completion, the ioctl returns to userspace with
inner.current_transaction cleared while the incoming synchronous
transaction remains queued in inner.work_list.
If that thread sends an outgoing synchronous BC_TRANSACTION before
draining its work queue, Thread::transaction_inner sets
inner.current_transaction to Some(t_out), and the read phase of the
same BINDER_WRITE_READ ioctl pops the pending incoming transaction and
overwrites inner.current_transaction with Some(t_in). This orphans
t_out from the sender's transaction stack so
Thread::unwind_transaction_stack aborts on exit, leaving a permanent
Arc<Thread> <-> DArc<Transaction> reference cycle that pins both
Arc<Process> instances and their mapped ShrinkablePageRange pages
after the processes exit.
Resolve the TODO in Thread::transaction_inner by rejecting outgoing
synchronous transactions with EINVAL when inner.work_list contains any
item where DeliverToRead::should_sync_wakeup() is true. Checking
should_sync_wakeup() matches C Binder's BINDER_WORK_TRANSACTION check
and ignores Node or NodeWrapper BR_INCREFS/BR_ACQUIRE items pushed
onto inner.work_list when Transaction::new translates flat binder
objects.
Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver")
Fixes: 8eb05e8e47f1 ("rust_binder: add TF_DEFER_COMPLETE flag for avoiding userspace roundtrip")
Cc: stable@vger.kernel.org
Signed-off-by: Chris Wiseman <chriswiseman@google.com>
---
drivers/android/binder/thread.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs
index f5965e838a96..3ca5c95695e4 100644
--- a/drivers/android/binder/thread.rs
+++ b/drivers/android/binder/thread.rs
@@ -1393,8 +1393,6 @@ fn transaction_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> BinderResu
info.to_pid = node_ref.node.owner.task.pid();
info.to_node_debug_id = node_ref.node.debug_id;
security::binder_transaction(&self.process.cred, &node_ref.node.owner.cred)?;
- // TODO: We need to ensure that there isn't a pending transaction in the work queue. How
- // could this happen?
let top = self.top_of_transaction_stack()?;
let list_completion = DTRWrap::arc_try_new(DeliverCode::new(
BR_TRANSACTION_COMPLETE,
@@ -1407,7 +1405,9 @@ fn transaction_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> BinderResu
// it with the new transaction.
{
let mut inner = self.inner.lock();
- if !transaction.is_stacked_on(&inner.current_transaction) {
+ if inner.work_list.iter().any(|w| w.should_sync_wakeup())
+ || !transaction.is_stacked_on(&inner.current_transaction)
+ {
binder_debug!(UserError, "got new transaction with bad transaction stack");
return Err(EINVAL.into());
}
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2/2] rust_binder: account for deferred work size in Thread::read() 2026-11-08 15:11 [PATCH 1/2] rust_binder: reject sync transaction with pending sync work Chris Wiseman @ 2026-11-08 15:11 ` Chris Wiseman 2026-11-09 19:42 ` Carlos Llamas 2026-11-09 19:15 ` [PATCH 1/2] rust_binder: reject sync transaction with pending sync work Carlos Llamas 1 sibling, 1 reply; 4+ messages in thread From: Chris Wiseman @ 2026-11-08 15:11 UTC (permalink / raw) To: Alice Ryhl, Carlos Llamas, Greg Kroah-Hartman Cc: Arve Hjønnevåg, Todd Kjos, Christian Brauner, Miguel Ojeda, rust-for-linux, linux-kernel, stable, Chris Wiseman When a looper thread replies with TF_DEFER_COMPLETE, its 4-byte BR_TRANSACTION_COMPLETE work item is queued via push_work_deferred() in inner.work_list without setting inner.process_work_list. On the next blocking BINDER_WRITE_READ ioctl, inner.should_use_process_work_queue() returns true and Thread::read() checks only whether the remaining output buffer can fit a single transaction (4-byte command code plus size_of::<uapi::binder_transaction_data_secctx>()). If userspace provides a read buffer with 76 bytes remaining after the initial 4-byte BR_NOOP placeholder (read_size = 80), Thread::read() calls Self::get_work(), sleeps in Process.ready_threads, and receives an incoming synchronous transaction queued behind the deferred BR_TRANSACTION_COMPLETE in inner.work_list. Iteration 1 of the loop writes the 4-byte BR_TRANSACTION_COMPLETE, reducing writer.len() to 72 bytes, and iteration 2 exits the loop without delivering the incoming transaction, leaving it stranded in the thread-local work_list. Add a size(&self) -> usize method to DeliverToRead returning the maximum number of bytes that do_work() may write to the user read buffer, and sum the sizes of all deferred items in inner.work_list when use_proc_queue is true. Include deferred_size on the right-hand side of the while condition in Thread::read() so the thread does not wait for global events when the output buffer lacks room for both its deferred work items and an incoming transaction, and subtract work.size() as each item is popped so subsequent loop iterations do not double-count items already written to writer. Suggested-by: Alice Ryhl <aliceryhl@google.com> Fixes: 8eb05e8e47f1 ("rust_binder: add TF_DEFER_COMPLETE flag for avoiding userspace roundtrip") Cc: stable@vger.kernel.org Signed-off-by: Chris Wiseman <chriswiseman@google.com> --- drivers/android/binder/freeze.rs | 4 +++ drivers/android/binder/node.rs | 8 +++++ drivers/android/binder/node/wrapper.rs | 4 +++ drivers/android/binder/rust_binder_main.rs | 8 +++++ drivers/android/binder/thread.rs | 34 ++++++++++++++++------ drivers/android/binder/transaction.rs | 4 +++ 6 files changed, 53 insertions(+), 9 deletions(-) diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs index 69e058e6d818..5103bfac781f 100644 --- a/drivers/android/binder/freeze.rs +++ b/drivers/android/binder/freeze.rs @@ -152,6 +152,10 @@ fn should_sync_wakeup(&self) -> bool { false } + fn size(&self) -> usize { + size_of::<u32>() + size_of::<BinderFrozenStateInfo>() + } + #[inline(never)] fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { seq_print!(m, "{}has frozen binder\n", prefix); diff --git a/drivers/android/binder/node.rs b/drivers/android/binder/node.rs index efdafc202e27..4f4c892cba58 100644 --- a/drivers/android/binder/node.rs +++ b/drivers/android/binder/node.rs @@ -754,6 +754,10 @@ fn should_sync_wakeup(&self) -> bool { false } + fn size(&self) -> usize { + 2 * (size_of::<u32>() + 2 * size_of::<u64>()) + } + #[inline(never)] fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { seq_print!( @@ -1149,6 +1153,10 @@ fn should_sync_wakeup(&self) -> bool { false } + fn size(&self) -> usize { + size_of::<u32>() + size_of::<u64>() + } + #[inline(never)] fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { let inner = self.inner.lock(); diff --git a/drivers/android/binder/node/wrapper.rs b/drivers/android/binder/node/wrapper.rs index edfbccf6e6da..37826349eafb 100644 --- a/drivers/android/binder/node/wrapper.rs +++ b/drivers/android/binder/node/wrapper.rs @@ -93,6 +93,10 @@ fn should_sync_wakeup(&self) -> bool { false } + fn size(&self) -> usize { + 2 * (size_of::<u32>() + 2 * size_of::<u64>()) + } + #[inline(never)] fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { seq_print!( diff --git a/drivers/android/binder/rust_binder_main.rs b/drivers/android/binder/rust_binder_main.rs index 56c786475237..361f3a987235 100644 --- a/drivers/android/binder/rust_binder_main.rs +++ b/drivers/android/binder/rust_binder_main.rs @@ -163,6 +163,10 @@ fn do_work( /// Generally only set to true for non-oneway transactions. fn should_sync_wakeup(&self) -> bool; + /// Returns the maximum size in bytes that [`DeliverToRead::do_work`] may write to the read + /// buffer. + fn size(&self) -> usize; + fn debug_print(&self, m: &SeqFile, prefix: &str, transaction_prefix: &str) -> Result; } @@ -279,6 +283,10 @@ fn should_sync_wakeup(&self) -> bool { false } + fn size(&self) -> usize { + size_of::<u32>() + } + fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { seq_print!(m, "{}", prefix); if self.skip.load(Relaxed) { diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs index 3ca5c95695e4..af7a340adcb3 100644 --- a/drivers/android/binder/thread.rs +++ b/drivers/android/binder/thread.rs @@ -1599,13 +1599,20 @@ fn read(self: &Arc<Self>, req: &mut BinderWriteRead, wait: bool) -> Result { UserSlice::new(UserPtr::from_addr(read_start as usize), read_len as usize).writer(), self, ); - let (in_pool, has_transaction, thread_todo, use_proc_queue) = { + let (in_pool, has_transaction, thread_todo, use_proc_queue, mut deferred_size) = { let inner = self.inner.lock(); + let use_proc_queue = inner.should_use_process_work_queue(); + let deferred_size = if use_proc_queue { + inner.work_list.iter().map(|w| w.size()).sum() + } else { + 0 + }; ( inner.is_looper(), inner.current_transaction.is_some(), !inner.work_list.is_empty(), - inner.should_use_process_work_queue(), + use_proc_queue, + deferred_size, ) }; @@ -1630,15 +1637,20 @@ fn read(self: &Arc<Self>, req: &mut BinderWriteRead, wait: bool) -> Result { // Loop doing work while there is room in the buffer. let initial_len = writer.len(); - while writer.len() >= size_of::<uapi::binder_transaction_data_secctx>() + 4 { + while writer.len() >= size_of::<uapi::binder_transaction_data_secctx>() + 4 + deferred_size + { match getter(self, wait && initial_len == writer.len()) { - Ok(Some(work)) => match work.into_arc().do_work(self, &mut writer) { - Ok(true) => {} - Ok(false) => break, - Err(err) => { - return Err(err); + Ok(Some(work)) => { + let work = work.into_arc(); + deferred_size = deferred_size.saturating_sub(work.size()); + match work.do_work(self, &mut writer) { + Ok(true) => {} + Ok(false) => break, + Err(err) => { + return Err(err); + } } - }, + } Ok(None) => { break; } @@ -1798,6 +1810,10 @@ fn should_sync_wakeup(&self) -> bool { false } + fn size(&self) -> usize { + size_of::<u32>() + } + fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { seq_print!( m, diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs index 48ac942bb75a..6909d7a8a219 100644 --- a/drivers/android/binder/transaction.rs +++ b/drivers/android/binder/transaction.rs @@ -762,6 +762,10 @@ fn should_sync_wakeup(&self) -> bool { !self.flags.is_oneway() } + fn size(&self) -> usize { + size_of::<u32>() + size_of::<BinderTransactionDataSecctx>() + } + fn debug_print(&self, m: &SeqFile, _prefix: &str, tprefix: &str) -> Result { self.debug_print_inner(m, tprefix); Ok(()) -- 2.56.0.317.g454f8302e1-goog ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] rust_binder: account for deferred work size in Thread::read() 2026-11-08 15:11 ` [PATCH 2/2] rust_binder: account for deferred work size in Thread::read() Chris Wiseman @ 2026-11-09 19:42 ` Carlos Llamas 0 siblings, 0 replies; 4+ messages in thread From: Carlos Llamas @ 2026-11-09 19:42 UTC (permalink / raw) To: Chris Wiseman Cc: Alice Ryhl, Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Christian Brauner, Miguel Ojeda, rust-for-linux, linux-kernel, stable On Sun, Nov 08, 2026 at 03:11:32PM +0000, Chris Wiseman wrote: > When a looper thread replies with TF_DEFER_COMPLETE, its 4-byte > BR_TRANSACTION_COMPLETE work item is queued via push_work_deferred() in > inner.work_list without setting inner.process_work_list. On the next > blocking BINDER_WRITE_READ ioctl, inner.should_use_process_work_queue() > returns true and Thread::read() checks only whether the remaining output > buffer can fit a single transaction (4-byte command code plus > size_of::<uapi::binder_transaction_data_secctx>()). > > If userspace provides a read buffer with 76 bytes remaining after the > initial 4-byte BR_NOOP placeholder (read_size = 80), Thread::read() > calls Self::get_work(), sleeps in Process.ready_threads, and receives an > incoming synchronous transaction queued behind the deferred > BR_TRANSACTION_COMPLETE in inner.work_list. Iteration 1 of the loop > writes the 4-byte BR_TRANSACTION_COMPLETE, reducing writer.len() to 72 > bytes, and iteration 2 exits the loop without delivering the incoming > transaction, leaving it stranded in the thread-local work_list. ok, so if userspace is setting TF_DEFER_COMPLETE, the read buffer size should reflect the possiblity of a BR_TRANSACTION_COMPLETE no? I'm not sure this is a driver issue. Maybe I'm missing something? I guess we are trying to match the behavior we have without the TF_DEFER_COMPLETE and using a smaller read buffer e.g. 65-bytes. I suppose this makes sense. > > Add a size(&self) -> usize method to DeliverToRead returning the maximum > number of bytes that do_work() may write to the user read buffer, and > sum the sizes of all deferred items in inner.work_list when > use_proc_queue is true. Include deferred_size on the right-hand side of > the while condition in Thread::read() so the thread does not wait for > global events when the output buffer lacks room for both its deferred > work items and an incoming transaction, and subtract work.size() as each > item is popped so subsequent loop iterations do not double-count items > already written to writer. > > Suggested-by: Alice Ryhl <aliceryhl@google.com> > Fixes: 8eb05e8e47f1 ("rust_binder: add TF_DEFER_COMPLETE flag for avoiding userspace roundtrip") > Cc: stable@vger.kernel.org > Signed-off-by: Chris Wiseman <chriswiseman@google.com> > --- > drivers/android/binder/freeze.rs | 4 +++ > drivers/android/binder/node.rs | 8 +++++ > drivers/android/binder/node/wrapper.rs | 4 +++ > drivers/android/binder/rust_binder_main.rs | 8 +++++ > drivers/android/binder/thread.rs | 34 ++++++++++++++++------ > drivers/android/binder/transaction.rs | 4 +++ > 6 files changed, 53 insertions(+), 9 deletions(-) > > diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs > index 69e058e6d818..5103bfac781f 100644 > --- a/drivers/android/binder/freeze.rs > +++ b/drivers/android/binder/freeze.rs > @@ -152,6 +152,10 @@ fn should_sync_wakeup(&self) -> bool { > false > } > > + fn size(&self) -> usize { > + size_of::<u32>() + size_of::<BinderFrozenStateInfo>() > + } > + > #[inline(never)] > fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { > seq_print!(m, "{}has frozen binder\n", prefix); > diff --git a/drivers/android/binder/node.rs b/drivers/android/binder/node.rs > index efdafc202e27..4f4c892cba58 100644 > --- a/drivers/android/binder/node.rs > +++ b/drivers/android/binder/node.rs > @@ -754,6 +754,10 @@ fn should_sync_wakeup(&self) -> bool { > false > } > > + fn size(&self) -> usize { > + 2 * (size_of::<u32>() + 2 * size_of::<u64>()) > + } > + > #[inline(never)] > fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { > seq_print!( > @@ -1149,6 +1153,10 @@ fn should_sync_wakeup(&self) -> bool { > false > } > > + fn size(&self) -> usize { > + size_of::<u32>() + size_of::<u64>() > + } > + > #[inline(never)] > fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { > let inner = self.inner.lock(); > diff --git a/drivers/android/binder/node/wrapper.rs b/drivers/android/binder/node/wrapper.rs > index edfbccf6e6da..37826349eafb 100644 > --- a/drivers/android/binder/node/wrapper.rs > +++ b/drivers/android/binder/node/wrapper.rs > @@ -93,6 +93,10 @@ fn should_sync_wakeup(&self) -> bool { > false > } > > + fn size(&self) -> usize { > + 2 * (size_of::<u32>() + 2 * size_of::<u64>()) > + } > + > #[inline(never)] > fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { > seq_print!( > diff --git a/drivers/android/binder/rust_binder_main.rs b/drivers/android/binder/rust_binder_main.rs > index 56c786475237..361f3a987235 100644 > --- a/drivers/android/binder/rust_binder_main.rs > +++ b/drivers/android/binder/rust_binder_main.rs > @@ -163,6 +163,10 @@ fn do_work( > /// Generally only set to true for non-oneway transactions. > fn should_sync_wakeup(&self) -> bool; > > + /// Returns the maximum size in bytes that [`DeliverToRead::do_work`] may write to the read > + /// buffer. > + fn size(&self) -> usize; > + > fn debug_print(&self, m: &SeqFile, prefix: &str, transaction_prefix: &str) -> Result; > } > > @@ -279,6 +283,10 @@ fn should_sync_wakeup(&self) -> bool { > false > } > > + fn size(&self) -> usize { > + size_of::<u32>() > + } > + > fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { > seq_print!(m, "{}", prefix); > if self.skip.load(Relaxed) { > diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs > index 3ca5c95695e4..af7a340adcb3 100644 > --- a/drivers/android/binder/thread.rs > +++ b/drivers/android/binder/thread.rs > @@ -1599,13 +1599,20 @@ fn read(self: &Arc<Self>, req: &mut BinderWriteRead, wait: bool) -> Result { > UserSlice::new(UserPtr::from_addr(read_start as usize), read_len as usize).writer(), > self, > ); > - let (in_pool, has_transaction, thread_todo, use_proc_queue) = { > + let (in_pool, has_transaction, thread_todo, use_proc_queue, mut deferred_size) = { > let inner = self.inner.lock(); > + let use_proc_queue = inner.should_use_process_work_queue(); > + let deferred_size = if use_proc_queue { > + inner.work_list.iter().map(|w| w.size()).sum() > + } else { > + 0 > + }; > ( > inner.is_looper(), > inner.current_transaction.is_some(), > !inner.work_list.is_empty(), > - inner.should_use_process_work_queue(), > + use_proc_queue, > + deferred_size, > ) > }; > > @@ -1630,15 +1637,20 @@ fn read(self: &Arc<Self>, req: &mut BinderWriteRead, wait: bool) -> Result { > > // Loop doing work while there is room in the buffer. > let initial_len = writer.len(); > - while writer.len() >= size_of::<uapi::binder_transaction_data_secctx>() + 4 { > + while writer.len() >= size_of::<uapi::binder_transaction_data_secctx>() + 4 + deferred_size > + { > match getter(self, wait && initial_len == writer.len()) { > - Ok(Some(work)) => match work.into_arc().do_work(self, &mut writer) { > - Ok(true) => {} > - Ok(false) => break, > - Err(err) => { > - return Err(err); > + Ok(Some(work)) => { > + let work = work.into_arc(); > + deferred_size = deferred_size.saturating_sub(work.size()); > + match work.do_work(self, &mut writer) { > + Ok(true) => {} > + Ok(false) => break, > + Err(err) => { > + return Err(err); > + } > } > - }, > + } > Ok(None) => { > break; > } > @@ -1798,6 +1810,10 @@ fn should_sync_wakeup(&self) -> bool { > false > } > > + fn size(&self) -> usize { > + size_of::<u32>() > + } > + > fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result { > seq_print!( > m, > diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs > index 48ac942bb75a..6909d7a8a219 100644 > --- a/drivers/android/binder/transaction.rs > +++ b/drivers/android/binder/transaction.rs > @@ -762,6 +762,10 @@ fn should_sync_wakeup(&self) -> bool { > !self.flags.is_oneway() > } > > + fn size(&self) -> usize { > + size_of::<u32>() + size_of::<BinderTransactionDataSecctx>() > + } > + > fn debug_print(&self, m: &SeqFile, _prefix: &str, tprefix: &str) -> Result { > self.debug_print_inner(m, tprefix); > Ok(()) > -- > 2.56.0.317.g454f8302e1-goog > It seems sashiko has found a legit issue, see: https://sashiko.dev/#/patchset/18e61a85b0d13e9c.3137322e32332e3235332e313736.9c152.0000sendgmr%40google.com -- Carlos Llamas ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] rust_binder: reject sync transaction with pending sync work 2026-11-08 15:11 [PATCH 1/2] rust_binder: reject sync transaction with pending sync work Chris Wiseman 2026-11-08 15:11 ` [PATCH 2/2] rust_binder: account for deferred work size in Thread::read() Chris Wiseman @ 2026-11-09 19:15 ` Carlos Llamas 1 sibling, 0 replies; 4+ messages in thread From: Carlos Llamas @ 2026-11-09 19:15 UTC (permalink / raw) To: Chris Wiseman Cc: Alice Ryhl, Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Christian Brauner, Miguel Ojeda, rust-for-linux, linux-kernel, stable On Sun, Nov 08, 2026 at 03:11:01PM +0000, Chris Wiseman wrote: > When a thread replies to a transaction with TF_DEFER_COMPLETE, its > BR_TRANSACTION_COMPLETE work item stays in inner.work_list while the > next blocking read waits in Process.ready_threads. A new incoming > synchronous transaction can then be queued behind that deferred > completion in inner.work_list. If the thread's read buffer only has > room for the deferred completion, the ioctl returns to userspace with > inner.current_transaction cleared while the incoming synchronous > transaction remains queued in inner.work_list. > > If that thread sends an outgoing synchronous BC_TRANSACTION before > draining its work queue, Thread::transaction_inner sets > inner.current_transaction to Some(t_out), and the read phase of the > same BINDER_WRITE_READ ioctl pops the pending incoming transaction and > overwrites inner.current_transaction with Some(t_in). This orphans > t_out from the sender's transaction stack so > Thread::unwind_transaction_stack aborts on exit, leaving a permanent > Arc<Thread> <-> DArc<Transaction> reference cycle that pins both > Arc<Process> instances and their mapped ShrinkablePageRange pages > after the processes exit. > > Resolve the TODO in Thread::transaction_inner by rejecting outgoing > synchronous transactions with EINVAL when inner.work_list contains any > item where DeliverToRead::should_sync_wakeup() is true. Checking > should_sync_wakeup() matches C Binder's BINDER_WORK_TRANSACTION check > and ignores Node or NodeWrapper BR_INCREFS/BR_ACQUIRE items pushed > onto inner.work_list when Transaction::new translates flat binder > objects. > > Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver") > Fixes: 8eb05e8e47f1 ("rust_binder: add TF_DEFER_COMPLETE flag for avoiding userspace roundtrip") > Cc: stable@vger.kernel.org > Signed-off-by: Chris Wiseman <chriswiseman@google.com> > --- > drivers/android/binder/thread.rs | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs > index f5965e838a96..3ca5c95695e4 100644 > --- a/drivers/android/binder/thread.rs > +++ b/drivers/android/binder/thread.rs > @@ -1393,8 +1393,6 @@ fn transaction_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> BinderResu > info.to_pid = node_ref.node.owner.task.pid(); > info.to_node_debug_id = node_ref.node.debug_id; > security::binder_transaction(&self.process.cred, &node_ref.node.owner.cred)?; > - // TODO: We need to ensure that there isn't a pending transaction in the work queue. How > - // could this happen? > let top = self.top_of_transaction_stack()?; > let list_completion = DTRWrap::arc_try_new(DeliverCode::new( > BR_TRANSACTION_COMPLETE, > @@ -1407,7 +1405,9 @@ fn transaction_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> BinderResu > // it with the new transaction. > { > let mut inner = self.inner.lock(); > - if !transaction.is_stacked_on(&inner.current_transaction) { > + if inner.work_list.iter().any(|w| w.should_sync_wakeup()) > + || !transaction.is_stacked_on(&inner.current_transaction) > + { > binder_debug!(UserError, "got new transaction with bad transaction stack"); > return Err(EINVAL.into()); > } > -- > 2.56.0.317.g454f8302e1-goog > If the issue happens when there is already a pending transaction, would it be better to relocate the new check to top_of_transaction_stack(), before we execute Transaction::new()? If possible, we can skip all the unwinding associated with this. If not.... Reviewed-by: Carlos Llamas <cmllamas@google.com> Thanks -- Carlos Llamas ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-09 19:42 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-11-08 15:11 [PATCH 1/2] rust_binder: reject sync transaction with pending sync work Chris Wiseman 2026-11-08 15:11 ` [PATCH 2/2] rust_binder: account for deferred work size in Thread::read() Chris Wiseman 2026-11-09 19:42 ` Carlos Llamas 2026-11-09 19:15 ` [PATCH 1/2] rust_binder: reject sync transaction with pending sync work Carlos Llamas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®