| From 2a74ccd1bcc170e66525f34e9de8652b57e2cf3d Mon Sep 17 00:00:00 2001 |
| From: Alice Ryhl <aliceryhl@google.com> |
| Date: Thu, 3 Sep 2026 11:36:03 +0000 |
| Subject: rust_binder: reschedule node refcount update on thread exit |
| |
| From: Alice Ryhl <aliceryhl@google.com> |
| |
| commit 2a74ccd1bcc170e66525f34e9de8652b57e2cf3d upstream. |
| |
| When a thread exits via BINDER_THREAD_EXIT, its pending work items are |
| cancelled. If a thread exits while holding a pending node refcount |
| increment (e.g. pushed as deferred work to that thread), the refcount |
| increment was previously dropped because Node::cancel() and |
| NodeWrapper::cancel() were no-ops. |
| |
| Dropping the refcount update leaves the node's delivery state and count |
| state desynchronized, and userspace will not receive the notification, |
| which can cause the node to never be freed from the process's nodes tree |
| when all external references are dropped. |
| |
| Fix this by implementing DeliverToRead::cancel() for Node and NodeWrapper |
| to move the pending refcount update to the process's work queue on thread |
| exit so another thread can deliver it to userspace. |
| |
| Cc: stable <stable@kernel.org> |
| Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver") |
| Signed-off-by: Alice Ryhl <aliceryhl@google.com> |
| Link: https://patch.msgid.link/20260903-binder-thread-exit-node-v1-1-be09ff14f6a4@google.com |
| [ Dropped first argument in calls to `push_work()` to adjust to the |
| existing form without mainline commit dbb17c9ea756 ("rust_binder: move |
| (e)poll wait queue to Process"). - Miguel ] |
| Signed-off-by: Miguel Ojeda <ojeda@kernel.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/android/binder/node.rs | 20 ++++++++++++++++--- |
| drivers/android/binder/node/wrapper.rs | 34 ++++++++++++++++++++++++++++++++- |
| 2 files changed, 50 insertions(+), 4 deletions(-) |
| |
| --- a/drivers/android/binder/node.rs |
| +++ b/drivers/android/binder/node.rs |
| @@ -49,9 +49,9 @@ pub(crate) struct CouldNotDeliverCritica |
| /// about to drop the weak reference, then the strong increment could be processed after the |
| /// other thread has already exited, which would be too late. |
| /// |
| -/// Note that trying to create a `ListArc` to the node can succeed even if `has_normal_push` is |
| +/// Note that trying to create a `ListArc` to the node can succeed even if `has_pushed_node` is |
| /// set. This is because another thread might just have popped the node from a todo list, but not |
| -/// yet called `do_work`. However, if `has_normal_push` is false, then creating a `ListArc` should |
| +/// yet called `do_work`. However, if `has_pushed_node` is false, then creating a `ListArc` should |
| /// always succeed. |
| /// |
| /// Like the other fields in `NodeInner`, the delivery state is protected by the process lock. |
| @@ -725,7 +725,21 @@ impl DeliverToRead for Node { |
| self.do_work_locked(writer, owner_inner) |
| } |
| |
| - fn cancel(self: DArc<Self>) {} |
| + fn cancel(self: DArc<Self>) { |
| + let _drop_outside_lock; |
| + let mut owner_inner = self.owner.inner.lock(); |
| + |
| + // We only do something on BINDER_THREAD_EXIT, not process exit. |
| + if owner_inner.is_dead { |
| + return; |
| + } |
| + |
| + // If BINDER_THREAD_EXIT is invoked on a thread with a pending node refcount update, we |
| + // should move ourselves to ensure the refcount update is still delivered. |
| + if let Some(node) = ListArc::try_from_arc_borrow(self.as_arc_borrow()) { |
| + _drop_outside_lock = owner_inner.push_work(node); |
| + } |
| + } |
| |
| fn should_sync_wakeup(&self) -> bool { |
| false |
| --- a/drivers/android/binder/node/wrapper.rs |
| +++ b/drivers/android/binder/node/wrapper.rs |
| @@ -57,7 +57,39 @@ impl DeliverToRead for NodeWrapper { |
| node.do_work_locked(writer, owner_inner) |
| } |
| |
| - fn cancel(self: DArc<Self>) {} |
| + fn cancel(self: DArc<Self>) { |
| + let _drop_outside_lock; |
| + let node = &self.node; |
| + let mut owner_inner = node.owner.inner.lock(); |
| + |
| + // We only do something on BINDER_THREAD_EXIT, not process exit. |
| + if owner_inner.is_dead { |
| + return; |
| + } |
| + |
| + // We transfer the responsibility of the node refcount update to the scheduled Node because |
| + // NodeWrapper has no way to re-create the ListArc. |
| + let inner = node.inner.access_mut(&mut owner_inner); |
| + |
| + let ds = &mut inner.delivery_state; |
| + assert!(ds.has_pushed_wrapper); |
| + assert!(ds.has_strong_zero2one); |
| + ds.has_pushed_wrapper = false; |
| + |
| + // We are changing the state to one where the Node is the strong zero2one update instead of |
| + // the wrapper. |
| + ds.has_weak_zero2one = false; |
| + |
| + if !ds.has_pushed_node { |
| + if let Some(node2) = ListArc::try_from_arc_borrow(node.as_arc_borrow()) { |
| + ds.has_pushed_node = true; |
| + _drop_outside_lock = owner_inner.push_work(node2); |
| + } else { |
| + // This can't actually happen. |
| + ds.has_strong_zero2one = false; |
| + } |
| + } |
| + } |
| |
| fn should_sync_wakeup(&self) -> bool { |
| false |