[PATCH] rust_binder: check context manager before creating node

[PATCH] rust_binder: check context manager before creating node

[Keshav Verma]

Rust Binder currently creates the Binder node before checking
whether a context manager is already registered. If a context manager already
exists, set_manager_node() returns -EBUSY after node state has already been
created.

Add a check before creating the node to match the C Binder ordering for
the common already registered case. Keep the final checks in set_manager_node()
so races with another caller are still handled after node creation.

Signed-off-by: Keshav Verma <iganschel@gmail.com>
---
 drivers/android/binder/context.rs | 20 ++++++++++++++++++++
 drivers/android/binder/process.rs |  1 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/android/binder/context.rs b/drivers/android/binder/context.rs
index ddddb66b3557..562fb339b31f 100644
--- a/drivers/android/binder/context.rs
+++ b/drivers/android/binder/context.rs
@@ -4,6 +4,7 @@
 
 use kernel::{
     alloc::kvec::KVVec,
+    cred::Credential,
     error::code::*,
     prelude::*,
     security,
@@ -107,6 +108,25 @@ pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Arc<Process>) {
         }
     }
 
+    pub(crate) fn check_manager(&self, cred: &Credential) -> Result {
+        let manager = self.manager.lock();
+        if manager.node.is_some() {
+            pr_warn!("BINDER_SET_CONTEXT_MGR already set");
+            return Err(EBUSY);
+        }
+        security::binder_set_context_mgr(cred)?;
+
+        // If the context manager has been set before, ensure that we use the same euid.
+        let caller_uid = Kuid::current_euid();
+        if let Some(ref uid) = manager.uid {
+            if *uid != caller_uid {
+                return Err(EPERM);
+            }
+        }
+
+        Ok(())
+    }
+
     pub(crate) fn set_manager_node(&self, node_ref: NodeRef) -> Result {
         let mut manager = self.manager.lock();
         if manager.node.is_some() {
diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 96b8440ceac6..d09facebddf6 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -741,6 +741,7 @@ fn set_as_manager(
         } else {
             (0, 0, 0)
         };
+        self.ctx.check_manager(&self.cred)?;
         let node_ref = self.get_node(ptr, cookie, flags as _, true, thread)?;
         let node = node_ref.node.clone();
         self.ctx.set_manager_node(node_ref)?;
-- 
2.39.5

Re: [PATCH] rust_binder: check context manager before creating node

[Greg Kroah-Hartman]

On Thu, Jun 18, 2026 at 03:50:30AM +0530, Keshav Verma wrote:
> Rust Binder currently creates the Binder node before checking
> whether a context manager is already registered. If a context manager already
> exists, set_manager_node() returns -EBUSY after node state has already been
> created.

Odd line-wrapping :(

> 
> Add a check before creating the node to match the C Binder ordering for
> the common already registered case. Keep the final checks in set_manager_node()
> so races with another caller are still handled after node creation.
> 
> Signed-off-by: Keshav Verma <iganschel@gmail.com>

What commit id does this "fix"?

> ---
>  drivers/android/binder/context.rs | 20 ++++++++++++++++++++
>  drivers/android/binder/process.rs |  1 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/android/binder/context.rs b/drivers/android/binder/context.rs
> index ddddb66b3557..562fb339b31f 100644
> --- a/drivers/android/binder/context.rs
> +++ b/drivers/android/binder/context.rs
> @@ -4,6 +4,7 @@
>  
>  use kernel::{
>      alloc::kvec::KVVec,
> +    cred::Credential,
>      error::code::*,
>      prelude::*,
>      security,
> @@ -107,6 +108,25 @@ pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Arc<Process>) {
>          }
>      }
>  
> +    pub(crate) fn check_manager(&self, cred: &Credential) -> Result {
> +        let manager = self.manager.lock();
> +        if manager.node.is_some() {
> +            pr_warn!("BINDER_SET_CONTEXT_MGR already set");

How can this be triggered?

thanks,

greg k-h

Re: [PATCH] rust_binder: check context manager before creating node

[Greg Kroah-Hartman]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Thu, Jun 18, 2026 at 04:49:31PM +0530, Keshav Verma wrote:
> Thanks, I will fix the line wrapping and add the appropriate Fixes tag in
> v2.

Also, you sent html email which the lists reject :(

> This can be triggered after the normal context manager has already been
> registered. A second process can open /dev/binder and call
> BINDER_SET_CONTEXT_MGR_EXT with a local BINDER_TYPE_BINDER object. The
> ioctl returns -EBUSY because the context manager already exists.
> 
> Repeating this with unique ptr/cookie values triggers the warning each time.
> 
> Since this is easy to trigger from userspace, should I drop the pr_warn!() from
> the check in v2 and just return -EBUSY there?

If userspace can trigger this, then please remove it so that it doesn't
cause a denial-of-service on the kernel log.

thanks,

greg k-h

Re: [PATCH] rust_binder: check context manager before creating node

[Miguel Ojeda]

On Thu, Jun 18, 2026 at 1:19 PM Keshav Verma <iganschel@gmail.com> wrote:
>
> Thanks, I will fix the line wrapping and add the appropriate Fixes tag in v2.

Please consider (depending on the hash that this fixes) whether it
needs Cc: stable@ too.

> Repeating this with unique ptr/cookie values triggers the warning each time.
>
> Since this is easy to trigger from userspace, should I drop the pr_warn!() from the check in v2 and just return -EBUSY there?

If it is something that is expected to be able to be triggered from
userspace, then printing is a bad idea, and at the very least it may
need to be `pr_warn_once!` if you somehow think it can be useful for
users.

Now, for things that are truly logic errors, i.e. things that in
principle should not be possible to trigger from userspace unless
there is a bug in the kernel, you may consider `pr_warn_once!` +
`debug_assert!(false);` in that path. But only if you are truly sure
that it not meant to be hit.

I hope that helps.

Cheers,
Miguel

Re: [PATCH] rust_binder: check context manager before creating node

[Alice Ryhl]

On Thu, Jun 18, 2026 at 12:27:49PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Jun 18, 2026 at 03:50:30AM +0530, Keshav Verma wrote:
> > Rust Binder currently creates the Binder node before checking
> > whether a context manager is already registered. If a context manager already
> > exists, set_manager_node() returns -EBUSY after node state has already been
> > created.
> 
> Odd line-wrapping :(
> 
> > 
> > Add a check before creating the node to match the C Binder ordering for
> > the common already registered case. Keep the final checks in set_manager_node()
> > so races with another caller are still handled after node creation.
> > 
> > Signed-off-by: Keshav Verma <iganschel@gmail.com>
> 
> What commit id does this "fix"?

There's no need to mark this as a fix. It's nothing more than a
fast-path that does less work when exiting on error. As the commit
message notes, the existing error check after get_node() is still needed
because otherwise two threads could invoke this in parallel leading to
multiple context managers.

> > +    pub(crate) fn check_manager(&self, cred: &Credential) -> Result {
> > +        let manager = self.manager.lock();
> > +        if manager.node.is_some() {
> > +            pr_warn!("BINDER_SET_CONTEXT_MGR already set");
> 
> How can this be triggered?

If two processes try to register themselves as the context manager of
this context, this occurs. The warning should probably be using
rate-limited printing.

Alice

[PATCH v2] rust_binder: check context manager before creating node

[Keshav Verma]

Rust Binder currently creates the Binder node before checking whether a
context manager is already registered. If a context manager already exists,
set_manager_node() returns -EBUSY after node state has already been created.

Add a check before creating the node to match the C Binder ordering for
the common already registered case. Keep the final checks in set_manager_node()
so races with another caller are still handled after node creation.

Signed-off-by: Keshav Verma <iganschel@gmail.com>
---
Changes in v2:
- Fix commit message line wrapping.
- Drop pr_warn!() from the pre-check since userspace can trigger it.

 drivers/android/binder/context.rs | 19 +++++++++++++++++++
 drivers/android/binder/process.rs |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/android/binder/context.rs b/drivers/android/binder/context.rs
index ddddb66b3557..f7ae84074f96 100644
--- a/drivers/android/binder/context.rs
+++ b/drivers/android/binder/context.rs
@@ -4,6 +4,7 @@
 
 use kernel::{
     alloc::kvec::KVVec,
+    cred::Credential,
     error::code::*,
     prelude::*,
     security,
@@ -107,6 +108,24 @@ pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Arc<Process>) {
         }
     }
 
+    pub(crate) fn check_manager(&self, cred: &Credential) -> Result {
+        let manager = self.manager.lock();
+        if manager.node.is_some() {
+            return Err(EBUSY);
+        }
+        security::binder_set_context_mgr(cred)?;
+
+        // If the context manager has been set before, ensure that we use the same euid.
+        let caller_uid = Kuid::current_euid();
+        if let Some(ref uid) = manager.uid {
+            if *uid != caller_uid {
+                return Err(EPERM);
+            }
+        }
+
+        Ok(())
+    }
+
     pub(crate) fn set_manager_node(&self, node_ref: NodeRef) -> Result {
         let mut manager = self.manager.lock();
         if manager.node.is_some() {
diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 96b8440ceac6..d09facebddf6 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -741,6 +741,7 @@ fn set_as_manager(
         } else {
             (0, 0, 0)
         };
+        self.ctx.check_manager(&self.cred)?;
         let node_ref = self.get_node(ptr, cookie, flags as _, true, thread)?;
         let node = node_ref.node.clone();
         self.ctx.set_manager_node(node_ref)?;
-- 
2.39.5