rust_binder: reject context manager self-transaction

PATCH Superseded Keshav Verma 3 participants 7 messages Jun 21 – Jul 2

Subsystems (2)

Rust Android drivers

lore

Share & tools

lore

Keshav Verma

author

2026-06-21 21:01 UTC #0 lore raw

PATCH 1/1 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 [email protected]

Fixes eafedbc7c050 ("rust_binder: add Rust Binder driver")

Signed-off-by Keshav Verma <[email protected]>

---
 drivers/android/binder/process.rs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/android/binder/process.rs +5 -1

index d09facebddf6..5befff9cf5b9 100644
@@ -901,7 +901,11 @@ pub(crate) fn insert_or_update_handle(
     pub(crate) fn get_transaction_node(&self, handle: u32) -> BinderResult<NodeRef> {
         // 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)?)
         }

Signature · 1 line

2.39.5

Keshav Verma

author

2026-06-22 14:58 UTC #1 lore raw

PATCH v2 1/1 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.

This applies to both synchronous and oneway transactions because both paths resolve the target through Process::get_transaction_node().

Cc [email protected]

Fixes eafedbc7c050 ("rust_binder: add Rust Binder driver")

Signed-off-by Keshav Verma <[email protected]>

---
Changes in v2:
- Compare the underlying OS process task instead of Rust Binder Process object.

 drivers/android/binder/process.rs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/android/binder/process.rs +5 -1

index d09facebddf6..13ecd3048b89 100644
@@ -901,7 +901,11 @@ pub(crate) fn insert_or_update_handle(
     pub(crate) fn get_transaction_node(&self, handle: u32) -> BinderResult<NodeRef> {
         // 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 self.task == node_ref.node.owner.task {
+                return Err(EINVAL.into());
+            }
+            Ok(node_ref)
         } else {
             Ok(self.get_node_from_handle(handle, true)?)
         }

Signature · 1 line

2.39.5

Alice Ryhl

maintainer

2026-06-23 11:37 UTC #2 lore raw

Quoted 11 lines · Keshav Verma

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().

Quoted 6 lines

Cc [email protected]
Fixes eafedbc7c050 ("rust_binder: add Rust Binder driver")
Signed-off-by Keshav Verma <[email protected]>
---
Changes in v2:
- Compare the underlying OS process task instead of Rust Binder Process object.

I would prefer to compare the Binder Process object. Rejecting transactions between different fds owned by the same process doesn't really have any benefit and makes fuzz testing much harder.

Alice

Carlos Llamas

maintainer

2026-06-25 00:38 UTC #3 lore raw

Quoted 19 lines · Alice Ryhl

Quoted 11 lines · Keshav Verma
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().
Quoted 6 lines
Cc [email protected]
Fixes eafedbc7c050 ("rust_binder: add Rust Binder driver")
Signed-off-by Keshav Verma <[email protected]>
---
Changes in v2:
- Compare the underlying OS process task instead of Rust Binder Process object.

Quoted 5 lines

I would prefer to compare the Binder Process object. Rejecting transactions between different fds owned by the same process doesn't really have any benefit and makes fuzz testing much harder.
Alice

Hey Alice,

The restrictions were added in the C version in order to patch vulnerabilities associated with this "self-transaction" behavior.
See: http://git.kernel.org/torvalds/c/4b836a1426cb

I haven't really looked much into this, but do we even need this for the Rust version? Is this even fixing anything at all?

--
Carlos Llamas

Alice Ryhl

maintainer

2026-06-25 05:52 UTC #4 lore raw

Quoted 27 lines · Carlos Llamas

Quoted 19 lines · Alice Ryhl
Quoted 11 lines · Keshav Verma
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().
Quoted 6 lines
Cc [email protected]
Fixes eafedbc7c050 ("rust_binder: add Rust Binder driver")
Signed-off-by Keshav Verma <[email protected]>
---
Changes in v2:
- Compare the underlying OS process task instead of Rust Binder Process object.
Quoted 5 lines
I would prefer to compare the Binder Process object. Rejecting transactions between different fds owned by the same process doesn't really have any benefit and makes fuzz testing much harder.
Alice

Quoted 8 lines

Hey Alice,
The restrictions were added in the C version in order to patch vulnerabilities associated with this "self-transaction" behavior.
See: http://git.kernel.org/torvalds/c/4b836a1426cb
I haven't really looked much into this, but do we even need this for the Rust version? Is this even fixing anything at all?

Even if there's no vulnerability, self-transactions are still very weird and may introduce edge cases I can't think of. I would prefer to reject them.

Alice

Keshav Verma

author

2026-06-25 10:40 UTC #5 lore raw

PATCH v3 1/1 rust_binder: reject context manager self-transaction

In reply to Keshav Verma

Rust binder resolved handle 0 to the context manager node, but it does not reject the case where the caller owns the same node.

This applies to both synchronous and oneway transactions because both paths resolve the target through Process::get_transaction_node().

Cc [email protected]

Fixes eafedbc7c050 ("rust_binder: add Rust Binder driver")

Signed-off-by Keshav Verma <[email protected]>

---
Changes in v3:
- Keep the check scoped to the Binder Process object, allowing transactions between different binder fds owned by the same OS process.

Changes in v2:
- Compare the underlying OS process task instead of Rust Binder Process object.

 drivers/android/binder/process.rs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/android/binder/process.rs +5 -1

index d09facebddf6..5befff9cf5b9 100644
@@ -901,7 +901,11 @@ pub(crate) fn insert_or_update_handle(
     pub(crate) fn get_transaction_node(&self, handle: u32) -> BinderResult<NodeRef> {
         // 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)?)
         }

Signature · 1 line

2.39.5

Alice Ryhl

maintainer Reviewed-by on v3 1/1

2026-07-02 10:00 UTC #6 lore raw

Quoted 8 lines · Keshav Verma

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.

Quoted 6 lines

This applies to both synchronous and oneway transactions because both paths resolve the target through Process::get_transaction_node().
Cc [email protected]
Fixes eafedbc7c050 ("rust_binder: add Rust Binder driver")
Signed-off-by Keshav Verma <[email protected]>

Reviewed-by Alice Ryhl <[email protected]>