Message-Id: <DG5CFX3ML5YL.2FE913F20LNPT@garyguo.net>
Date: Tue, 03 Feb 2026 12:59:45 +0000
From: "Gary Guo" <gary@...yguo.net>
To: <jongan.kim@....com>, <a.hindborg@...nel.org>, <aliceryhl@...gle.com>,
 <arve@...roid.com>, <bjorn3_gh@...tonmail.com>, <boqun.feng@...il.com>,
 <brauner@...nel.org>, <cmllamas@...gle.com>, <dakr@...nel.org>,
 <daniel.almeida@...labora.com>, <gary@...yguo.net>,
 <gregkh@...uxfoundation.org>, <tamird@...il.com>, <tkjos@...roid.com>,
 <tmgross@...ch.edu>, <viresh.kumar@...aro.org>, <vitaly.wool@...sulko.se>,
 <yury.norov@...il.com>, <lossin@...nel.org>, <ojeda@...nel.org>
Cc: <heesu0025.kim@....com>, <ht.hong@....com>, <jungsu.hwang@....com>,
 <kernel-team@...roid.com>, <linux-kernel@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>, <sanghun.lee@....com>,
 <seulgi.lee@....com>, <sunghoon.kim@....com>
Subject: Re: [PATCH v3 3/3] rust_binder: handle PID namespace conversion for
 freeze operation

On Tue Feb 3, 2026 at 6:59 AM GMT, jongan.kim wrote:
> From: HeeSu Kim <heesu0025.kim@....com>
>
> Port PID namespace conversion logic from C binder to the Rust
> implementation.
>
> Without namespace conversion, freeze operations from non-init namespaces
> can match wrong processes due to PID collision. This adds proper
> conversion to ensure freeze operations target the correct process.
>
> Signed-off-by: HeeSu Kim <heesu0025.kim@....com>
> ---
> v2 -> v3:
> - Use task::Pid typedef instead of u32/i32
> - Use PidNamespace::init_ns() instead of init_pid_ns() 
> - Compare PidNamespace directly with == instead of raw pointers
> - Use Pid::find_vpid() and pid.pid_task() (dropped _with_guard suffix)
> - Fix rustfmt import ordering (rcu before Arc)
> - Rename TaskPid alias to PidT for clearer pid_t type indication
> - Use task.group_leader().pid() instead of tgid_nr_ns() for consistency with C
>
>  drivers/android/binder/process.rs | 37 +++++++++++++++++++++++++++----
>  1 file changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
> index 132055b4790f..ea30bfac2e0b 100644
> --- a/drivers/android/binder/process.rs
> +++ b/drivers/android/binder/process.rs
> @@ -22,6 +22,8 @@
>      id_pool::IdPool,
>      list::{List, ListArc, ListArcField, ListLinks},
>      mm,
> +    pid::Pid,
> +    pid_namespace::PidNamespace,
>      prelude::*,
>      rbtree::{self, RBTree, RBTreeNode, RBTreeNodeReservation},
>      seq_file::SeqFile,
> @@ -29,9 +31,9 @@
>      sync::poll::PollTable,
>      sync::{
>          lock::{spinlock::SpinLockBackend, Guard},
> -        Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, SpinLock, UniqueArc,
> +        rcu, Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, SpinLock, UniqueArc,
>      },
> -    task::Task,
> +    task::{Pid as PidT, Task},
>      types::ARef,
>      uaccess::{UserSlice, UserSliceReader},
>      uapi,
> @@ -1498,17 +1500,42 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
>      }
>  }
>  
> +/// Convert a PID from the current namespace to the global (init) namespace.
> +fn convert_to_init_ns_tgid(pid: PidT) -> Result<PidT> {
> +    let current = kernel::current!();
> +    let init_ns = PidNamespace::init_ns();
> +
> +    if current.active_pid_ns() == Some(init_ns) {
> +        // Already in init namespace.
> +        return Ok(pid);
> +    }
> +
> +    if pid == 0 {
> +        return Err(EINVAL);
> +    }
> +
> +    let rcu_guard = rcu::read_lock();
> +
> +    let pid_struct = Pid::find_vpid(pid, &rcu_guard).ok_or(ESRCH)?;
> +    let task = pid_struct.pid_task(&rcu_guard).ok_or(ESRCH)?;
> +
> +    Ok(task.group_leader().pid())
> +}
> +
>  fn get_frozen_status(data: UserSlice) -> Result {
>      let (mut reader, mut writer) = data.reader_writer();
>  
>      let mut info = reader.read::<BinderFrozenStatusInfo>()?;
> +
> +    let init_ns_pid = convert_to_init_ns_tgid(info.pid as PidT)?;
> +
>      info.sync_recv = 0;
>      info.async_recv = 0;
>      let mut found = false;
>  
>      for ctx in crate::context::get_all_contexts()? {
>          ctx.for_each_proc(|proc| {
> -            if proc.task.pid() == info.pid as _ {
> +            if proc.task.pid() == init_ns_pid as _ {
>                  found = true;
>                  let inner = proc.inner.lock();
>                  let txns_pending = inner.txns_pending_locked();
> @@ -1530,13 +1557,15 @@ fn get_frozen_status(data: UserSlice) -> Result {
>  fn ioctl_freeze(reader: &mut UserSliceReader) -> Result {
>      let info = reader.read::<BinderFreezeInfo>()?;
>  
> +    let init_ns_pid = convert_to_init_ns_tgid(info.pid as PidT)?;
> +
>      // Very unlikely for there to be more than 3, since a process normally uses at most binder and
>      // hwbinder.
>      let mut procs = KVec::with_capacity(3, GFP_KERNEL)?;
>  
>      let ctxs = crate::context::get_all_contexts()?;
>      for ctx in ctxs {
> -        for proc in ctx.get_procs_with_pid(info.pid as i32)? {
> +        for proc in ctx.get_procs_with_pid(init_ns_pid)? {

I think this function already has `ARef<Task>` for all the process, so it feels
to me that the search should be simply based on task, rather than PID.

I.e. instead of pid_t -> struct pid -> struct tasks -> pid_t and then search
with it, we first get `Task` from VPID, and then search directly with it. This
would also make it no longer necessary to have the init namespace check.

(BTW, the current impl looks quite inefficient, get_procs_with_pid returns a vec
which is pushed again to a vec, perhaps using a callback or simply passing in a
`&mut Vec` is better?)

Best,
Gary

>              procs.push(proc, GFP_KERNEL)?;
>          }
>      }

Powered by blists - more mailing lists