mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: <jongan.kim@lge.com>, <a.hindborg@kernel.org>,
	<aliceryhl@google.com>, <arve@android.com>,
	<bjorn3_gh@protonmail.com>, <boqun.feng@gmail.com>,
	<brauner@kernel.org>, <cmllamas@google.com>, <dakr@kernel.org>,
	<daniel.almeida@collabora.com>, <gary@garyguo.net>,
	<gregkh@linuxfoundation.org>, <tamird@gmail.com>,
	<tkjos@android.com>, <tmgross@umich.edu>,
	<viresh.kumar@linaro.org>, <vitaly.wool@konsulko.se>,
	<yury.norov@gmail.com>, <lossin@kernel.org>, <ojeda@kernel.org>
Cc: <heesu0025.kim@lge.com>, <ht.hong@lge.com>,
	<jungsu.hwang@lge.com>, <kernel-team@android.com>,
	<linux-kernel@vger.kernel.org>, <rust-for-linux@vger.kernel.org>,
	<sanghun.lee@lge.com>, <seulgi.lee@lge.com>,
	<sunghoon.kim@lge.com>
Subject: Re: [PATCH v3 3/3] rust_binder: handle PID namespace conversion for freeze operation
Date: Tue, 03 Feb 2026 12:59:45 +0000	[thread overview]
Message-ID: <DG5CFX3ML5YL.2FE913F20LNPT@garyguo.net> (raw)
In-Reply-To: <20260203065928.4736-4-jongan.kim@lge.com>

On Tue Feb 3, 2026 at 6:59 AM GMT, jongan.kim wrote:
> From: HeeSu Kim <heesu0025.kim@lge.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@lge.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)?;
>          }
>      }


  reply	other threads:[~2026-02-03 12:59 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-03  6:59 [PATCH v3 0/3] binder: " jongan.kim
2026-02-03  6:59 ` [PATCH v3 1/3] " jongan.kim
2026-02-03 20:38   ` Yury Norov
2026-02-04  9:05     ` jongan.kim
2026-02-04 17:04       ` Yury Norov
2026-02-05  5:30         ` jongan.kim
2026-02-03  6:59 ` [PATCH v3 2/3] rust: pid: add Pid abstraction and init_ns helper jongan.kim
2026-02-03 13:01   ` Gary Guo
2026-02-03  6:59 ` [PATCH v3 3/3] rust_binder: handle PID namespace conversion for freeze operation jongan.kim
2026-02-03 12:59   ` Gary Guo [this message]
2026-02-04  9:11     ` jongan.kim
2026-02-04 10:50       ` Alice Ryhl
2026-02-05  5:01         ` jongan.kim
2026-02-05  8:20           ` Alice Ryhl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DG5CFX3ML5YL.2FE913F20LNPT@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=arve@android.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=brauner@kernel.org \
    --cc=cmllamas@google.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heesu0025.kim@lge.com \
    --cc=ht.hong@lge.com \
    --cc=jongan.kim@lge.com \
    --cc=jungsu.hwang@lge.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sanghun.lee@lge.com \
    --cc=seulgi.lee@lge.com \
    --cc=sunghoon.kim@lge.com \
    --cc=tamird@gmail.com \
    --cc=tkjos@android.com \
    --cc=tmgross@umich.edu \
    --cc=viresh.kumar@linaro.org \
    --cc=vitaly.wool@konsulko.se \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®