[PATCH] rust_binder: do not query current thread for all ioctls

[PATCH] rust_binder: do not query current thread for all ioctls

[Alice Ryhl]

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 <aliceryhl@google.com>
---
 drivers/android/binder/process.rs | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 1778628d8acd..5372bfbd93b3 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -1662,6 +1662,10 @@ fn ioctl_write_only(
         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()?),
@@ -1673,7 +1677,6 @@ fn ioctl_write_only(
             uapi::BINDER_ENABLE_ONEWAY_SPAM_DETECTION => {
                 this.set_oneway_spam_detection_enabled(reader.read()?)
             }
-            uapi::BINDER_FREEZE => ioctl_freeze(reader)?,
             _ => return Err(EINVAL),
         }
         Ok(())
@@ -1688,15 +1691,16 @@ fn ioctl_write_read(
         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(())

---
base-commit: 2cedf2272f1bb42471e646868ac572cc5752bd91
change-id: 20260727-binder-cur-thread-974d7ab2cbbc

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

Re: [PATCH] rust_binder: do not query current thread for all ioctls

[Alice Ryhl]

On Mon, Jul 27, 2026 at 2:29 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> 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 <aliceryhl@google.com>

Oh I forgot to mark this as Fixes

Cc: stable@vger.kernel.org
Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver")

Greg let me know if you need a version for this.

Re: [PATCH] rust_binder: do not query current thread for all ioctls

[Carlos Llamas]

On Mon, Jul 27, 2026 at 12:28:59PM +0000, Alice Ryhl wrote:
> 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 <aliceryhl@google.com>
> ---
LGTM,

Acked-by: Carlos Llamas <cmllamas@google.com>