public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] binder: handle PID namespace conversion for freeze operation
@ 2026-01-29  8:41 jongan.kim
  2026-01-29  8:41 ` [PATCH v2 1/3] " jongan.kim
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: jongan.kim @ 2026-01-29  8:41 UTC (permalink / raw)
  To: aliceryhl, arve, brauner, cmllamas, gregkh, tkjos, ojeda,
	boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, dakr,
	yury.norov, vitaly.wool, tamird, viresh.kumar, daniel.almeida
  Cc: linux-kernel, rust-for-linux, jongan.kim, heesu0025.kim, ht.hong,
	jungsu.hwang, kernel-team, sanghun.lee, seulgi.lee, sunghoon.kim

From: JongAn Kim <jongan.kim@lge.com>

This patch series fixes PID namespace handling in binder's freeze operation
for both C and Rust implementations.

This series addresses the issue by:

1. Patch 1/3: Fixes the C binder implementation by adding PID namespace
   conversion logic. It converts the caller's PID from their namespace to
   the init namespace before matching against binder_proc->pid, ensuring
   correct process identification.

2. Patch 2/3: Adds Rust abstractions for PID handling, including:
   - New Pid abstraction wrapping kernel's struct pid
   - find_vpid_with_guard() and pid_task_with_guard() functions with RCU
     protection
   - init_pid_ns() helper to access the init PID namespace
   These abstractions provide safe Rust interfaces with lifetime-bounded
   references tied to RCU guards for memory safety.

3. Patch 3/3: Ports the PID namespace conversion logic to the Rust binder
   implementation, using the new abstractions to ensure freeze operations
   from non-init namespaces target the correct process.

This ensures consistent and correct PID handling across both C and Rust
binder implementations when freeze operations occur in containerized
environments.

v1 : https://lore.kernel.org/lkml/20251203024140.175952-1-jongan.kim@lge.com/T/#u

v1 -> v2 changes:
- add two more patches to implement the same logic in Rust binder

HeeSu Kim (2):
  rust: pid: add Pid abstraction and init_pid_ns helper
  rust_binder: handle PID namespace conversion for freeze operation

JongAn Kim (1):
  binder: handle PID namespace conversion for freeze operation

 drivers/android/binder.c          |  52 +++++++++++++++-
 drivers/android/binder/process.rs |  40 +++++++++++-
 rust/kernel/lib.rs                |   1 +
 rust/kernel/pid.rs                | 100 ++++++++++++++++++++++++++++++
 rust/kernel/pid_namespace.rs      |   9 +++
 5 files changed, 196 insertions(+), 6 deletions(-)
 create mode 100644 rust/kernel/pid.rs

-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/3] binder: handle PID namespace conversion for freeze operation
  2026-01-29  8:41 [PATCH v2 0/3] binder: handle PID namespace conversion for freeze operation jongan.kim
@ 2026-01-29  8:41 ` jongan.kim
  2026-01-29 10:41   ` Alice Ryhl
  2026-01-29  8:41 ` [PATCH v2 2/3] rust: pid: add Pid abstraction and init_pid_ns helper jongan.kim
  2026-01-29  8:41 ` [PATCH v2 3/3] rust_binder: handle PID namespace conversion for freeze operation jongan.kim
  2 siblings, 1 reply; 12+ messages in thread
From: jongan.kim @ 2026-01-29  8:41 UTC (permalink / raw)
  To: aliceryhl, arve, brauner, cmllamas, gregkh, tkjos, ojeda,
	boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, dakr,
	yury.norov, vitaly.wool, tamird, viresh.kumar, daniel.almeida
  Cc: linux-kernel, rust-for-linux, jongan.kim, heesu0025.kim, ht.hong,
	jungsu.hwang, kernel-team, sanghun.lee, seulgi.lee, sunghoon.kim

From: JongAn Kim <jongan.kim@lge.com>

Currently, when a freeze is attempted from a non-init PID namespace,
there is a possibility that the wrong process in the init namespace
may be frozen due to PID collision across namespaces.

For example, if a container with PID namespace has a process with
PID 100 (which maps to PID 5000 in init namespace), attempting to
freeze PID 100 from the container could incorrectly match a different
process with PID 100 in the init namespace.

This patch fixes the issue by:
1. Converting the caller's PID from their namespace to init namespace
2. Matching against binder_proc->pid (which stores init namespace TGID)
3. Returning -EINVAL for invalid PIDs and -ESRCH for not-found processes

This change ensures correct PID handling when binder freeze occurs in
non-init PID namespace.

Signed-off-by: JongAn Kim <jongan.kim@lge.com>
---
 drivers/android/binder.c | 52 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 535fc881c8da..4695e459c924 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5609,6 +5609,40 @@ static bool binder_txns_pending_ilocked(struct binder_proc *proc)
 	return false;
 }
 
+/**
+ * binder_convert_to_init_ns_tgid() - Convert pid to global pid(init namespace)
+ * @pid:    pid from user space
+ *
+ * Converts a process ID (TGID) from the caller's PID namespace to the
+ * corresponding TGID in the init namespace.
+ *
+ * Return: On success, returns TGID in init namespace (positive value).
+ *         On error, returns -EINVAL if pid <= 0, or -ESRCH if process
+ *         not found or not visible in init namespace.
+ */
+static int binder_convert_to_init_ns_tgid(u32 pid)
+{
+	struct task_struct *task;
+	int init_ns_pid;
+
+	/* already in init namespace */
+	if (task_is_in_init_pid_ns(current))
+		return pid;
+
+	if (pid == 0)
+		return -EINVAL;
+
+	rcu_read_lock();
+	task = pid_task(find_vpid(pid), PIDTYPE_PID);
+	init_ns_pid = task ? task_tgid_nr_ns(task, &init_pid_ns) : -ESRCH;
+	rcu_read_unlock();
+
+	if (!init_ns_pid)
+		return -ESRCH;
+
+	return init_ns_pid;
+}
+
 static void binder_add_freeze_work(struct binder_proc *proc, bool is_frozen)
 {
 	struct binder_node *prev = NULL;
@@ -5717,13 +5751,18 @@ static int binder_ioctl_get_freezer_info(
 	struct binder_proc *target_proc;
 	bool found = false;
 	__u32 txns_pending;
+	int init_ns_pid = 0;
 
 	info->sync_recv = 0;
 	info->async_recv = 0;
 
+	init_ns_pid = binder_convert_to_init_ns_tgid(info->pid);
+	if (init_ns_pid < 0)
+		return init_ns_pid;
+
 	mutex_lock(&binder_procs_lock);
 	hlist_for_each_entry(target_proc, &binder_procs, proc_node) {
-		if (target_proc->pid == info->pid) {
+		if (target_proc->pid == init_ns_pid) {
 			found = true;
 			binder_inner_proc_lock(target_proc);
 			txns_pending = binder_txns_pending_ilocked(target_proc);
@@ -5869,6 +5908,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		struct binder_freeze_info info;
 		struct binder_proc **target_procs = NULL, *target_proc;
 		int target_procs_count = 0, i = 0;
+		int init_ns_pid = 0;
 
 		ret = 0;
 
@@ -5877,9 +5917,15 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			goto err;
 		}
 
+		init_ns_pid = binder_convert_to_init_ns_tgid(info.pid);
+		if (init_ns_pid < 0) {
+			ret = init_ns_pid;
+			goto err;
+		}
+
 		mutex_lock(&binder_procs_lock);
 		hlist_for_each_entry(target_proc, &binder_procs, proc_node) {
-			if (target_proc->pid == info.pid)
+			if (target_proc->pid == init_ns_pid)
 				target_procs_count++;
 		}
 
@@ -5900,7 +5946,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		}
 
 		hlist_for_each_entry(target_proc, &binder_procs, proc_node) {
-			if (target_proc->pid != info.pid)
+			if (target_proc->pid != init_ns_pid)
 				continue;
 
 			binder_inner_proc_lock(target_proc);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/3] rust: pid: add Pid abstraction and init_pid_ns helper
  2026-01-29  8:41 [PATCH v2 0/3] binder: handle PID namespace conversion for freeze operation jongan.kim
  2026-01-29  8:41 ` [PATCH v2 1/3] " jongan.kim
@ 2026-01-29  8:41 ` jongan.kim
  2026-01-29 10:32   ` Alice Ryhl
  2026-01-29 14:33   ` Gary Guo
  2026-01-29  8:41 ` [PATCH v2 3/3] rust_binder: handle PID namespace conversion for freeze operation jongan.kim
  2 siblings, 2 replies; 12+ messages in thread
From: jongan.kim @ 2026-01-29  8:41 UTC (permalink / raw)
  To: aliceryhl, arve, brauner, cmllamas, gregkh, tkjos, ojeda,
	boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, dakr,
	yury.norov, vitaly.wool, tamird, viresh.kumar, daniel.almeida
  Cc: linux-kernel, rust-for-linux, jongan.kim, heesu0025.kim, ht.hong,
	jungsu.hwang, kernel-team, sanghun.lee, seulgi.lee, sunghoon.kim

From: HeeSu Kim <heesu0025.kim@lge.com>

Add a new Pid abstraction in rust/kernel/pid.rs that wraps the
kernel's struct pid and provides safe Rust interfaces for:
- find_vpid_with_guard: Find a pid by number under RCU protection
- pid_task_with_guard: Get the task associated with a pid under RCU
  protection

Also add init_pid_ns() helper function to pid_namespace.rs to get
a reference to the init PID namespace.

These abstractions use lifetime-bounded references tied to RCU guards
to ensure memory safety when accessing RCU-protected data structures.

Signed-off-by: HeeSu Kim <heesu0025.kim@lge.com>
---
 rust/kernel/lib.rs           |   1 +
 rust/kernel/pid.rs           | 100 +++++++++++++++++++++++++++++++++++
 rust/kernel/pid_namespace.rs |   9 ++++
 3 files changed, 110 insertions(+)
 create mode 100644 rust/kernel/pid.rs

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index f812cf120042..60a518d65d0e 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -122,6 +122,7 @@
 pub mod page;
 #[cfg(CONFIG_PCI)]
 pub mod pci;
+pub mod pid;
 pub mod pid_namespace;
 pub mod platform;
 pub mod prelude;
diff --git a/rust/kernel/pid.rs b/rust/kernel/pid.rs
new file mode 100644
index 000000000000..00989b953c23
--- /dev/null
+++ b/rust/kernel/pid.rs
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Process identifiers (PIDs).
+//!
+//! C header: [`include/linux/pid.h`](srctree/include/linux/pid.h)
+
+use crate::{bindings, ffi::c_int, sync::rcu, task::Task, types::Opaque};
+
+/// Wraps the kernel's `struct pid`.
+///
+/// This structure represents the Rust abstraction for a C `struct pid`.
+/// A `Pid` represents a process identifier that can be looked up in different
+/// PID namespaces.
+#[repr(transparent)]
+pub struct Pid {
+    inner: Opaque<bindings::pid>,
+}
+
+impl Pid {
+    /// Returns a raw pointer to the inner C struct.
+    #[inline]
+    pub fn as_ptr(&self) -> *mut bindings::pid {
+        self.inner.get()
+    }
+
+    /// Finds a `struct pid` by its pid number within the current task's PID namespace.
+    ///
+    /// Returns `None` if no such pid exists.
+    ///
+    /// The returned reference is only valid for the duration of the RCU read-side
+    /// critical section represented by the `rcu::Guard`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::pid::Pid;
+    /// use kernel::sync::rcu;
+    ///
+    /// let guard = rcu::read_lock();
+    /// if let Some(pid) = Pid::find_vpid_with_guard(1, &guard) {
+    ///     pr_info!("Found pid 1\n");
+    /// }
+    /// ```
+    ///
+    /// Returns `None` for non-existent PIDs:
+    ///
+    /// ```
+    /// use kernel::pid::Pid;
+    /// use kernel::sync::rcu;
+    ///
+    /// let guard = rcu::read_lock();
+    /// // PID 0 (swapper/idle) is not visible via find_vpid.
+    /// assert!(Pid::find_vpid_with_guard(0, &guard).is_none());
+    /// ```
+    #[inline]
+    pub fn find_vpid_with_guard<'a>(nr: i32, _rcu_guard: &'a rcu::Guard) -> Option<&'a Self> {
+        // SAFETY: Called under RCU protection as guaranteed by the Guard reference.
+        let ptr = unsafe { bindings::find_vpid(nr as c_int) };
+        if ptr.is_null() {
+            None
+        } else {
+            // SAFETY: `find_vpid` returns a valid pointer under RCU protection,
+            // and `Pid` is `#[repr(transparent)]` over `bindings::pid`.
+            Some(unsafe { &*(ptr as *const Self) })
+        }
+    }
+
+    /// Gets the task associated with this PID.
+    ///
+    /// Returns `None` if no task is associated with this PID.
+    ///
+    /// The returned reference is only valid for the duration of the RCU read-side
+    /// critical section represented by the `rcu::Guard`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::pid::Pid;
+    /// use kernel::sync::rcu;
+    ///
+    /// let guard = rcu::read_lock();
+    /// if let Some(pid) = Pid::find_vpid_with_guard(1, &guard) {
+    ///     if let Some(task) = pid.pid_task_with_guard(&guard) {
+    ///         pr_info!("Found task for pid 1\n");
+    ///     }
+    /// }
+    /// ```
+    #[inline]
+    pub fn pid_task_with_guard<'a>(&'a self, _rcu_guard: &'a rcu::Guard) -> Option<&'a Task> {
+        // SAFETY: Called under RCU protection as guaranteed by the Guard reference.
+        let task_ptr = unsafe { bindings::pid_task(self.as_ptr(), bindings::pid_type_PIDTYPE_PID) };
+        if task_ptr.is_null() {
+            None
+        } else {
+            // SAFETY: `pid_task` returns a valid pointer under RCU protection,
+            // and `Task` is `#[repr(transparent)]` over `bindings::task_struct`.
+            Some(unsafe { &*task_ptr.cast() })
+        }
+    }
+}
diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
index 979a9718f153..6029e3e120d0 100644
--- a/rust/kernel/pid_namespace.rs
+++ b/rust/kernel/pid_namespace.rs
@@ -63,3 +63,12 @@ unsafe impl Send for PidNamespace {}
 // SAFETY: It's OK to access `PidNamespace` through shared references from other threads because
 // we're either accessing properties that don't change or that are properly synchronised by C code.
 unsafe impl Sync for PidNamespace {}
+
+/// Returns a reference to the init PID namespace.
+///
+/// This is the root PID namespace that exists throughout the lifetime of the kernel.
+#[inline]
+pub fn init_pid_ns() -> &'static PidNamespace {
+    // SAFETY: `init_pid_ns` is a global static that is valid for the lifetime of the kernel.
+    unsafe { PidNamespace::from_ptr(core::ptr::addr_of!(bindings::init_pid_ns)) }
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/3] rust_binder: handle PID namespace conversion for freeze operation
  2026-01-29  8:41 [PATCH v2 0/3] binder: handle PID namespace conversion for freeze operation jongan.kim
  2026-01-29  8:41 ` [PATCH v2 1/3] " jongan.kim
  2026-01-29  8:41 ` [PATCH v2 2/3] rust: pid: add Pid abstraction and init_pid_ns helper jongan.kim
@ 2026-01-29  8:41 ` jongan.kim
  2026-01-29 10:35   ` Alice Ryhl
  2 siblings, 1 reply; 12+ messages in thread
From: jongan.kim @ 2026-01-29  8:41 UTC (permalink / raw)
  To: aliceryhl, arve, brauner, cmllamas, gregkh, tkjos, ojeda,
	boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, dakr,
	yury.norov, vitaly.wool, tamird, viresh.kumar, daniel.almeida
  Cc: linux-kernel, rust-for-linux, jongan.kim, heesu0025.kim, ht.hong,
	jungsu.hwang, kernel-team, sanghun.lee, seulgi.lee, sunghoon.kim

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>
---
 drivers/android/binder/process.rs | 40 ++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 132055b4790f..41b89accea6a 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::init_pid_ns,
     prelude::*,
     rbtree::{self, RBTree, RBTreeNode, RBTreeNodeReservation},
     seq_file::SeqFile,
@@ -29,7 +31,7 @@
     sync::poll::PollTable,
     sync::{
         lock::{spinlock::SpinLockBackend, Guard},
-        Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, SpinLock, UniqueArc,
+        Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, rcu, SpinLock, UniqueArc,
     },
     task::Task,
     types::ARef,
@@ -1498,17 +1500,47 @@ 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: u32) -> Result<i32> {
+    let current = kernel::current!();
+    let init_ns = init_pid_ns();
+
+    if current.active_pid_ns().map(|ns| ns.as_ptr()) == Some(init_ns.as_ptr()) {
+        // Already in init namespace.
+        return Ok(pid as i32);
+    }
+
+    if pid == 0 {
+        return Err(EINVAL);
+    }
+
+    let rcu_guard = rcu::read_lock();
+
+    let pid_struct = Pid::find_vpid_with_guard(pid as i32, &rcu_guard).ok_or(ESRCH)?;
+    let task = pid_struct.pid_task_with_guard(&rcu_guard).ok_or(ESRCH)?;
+    let init_ns_pid = task.tgid_nr_ns(Some(init_ns));
+
+    if init_ns_pid == 0 {
+        return Err(ESRCH);
+    }
+
+    Ok(init_ns_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)?;
+
     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 +1562,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)?;
+
     // 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)? {
             procs.push(proc, GFP_KERNEL)?;
         }
     }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/3] rust: pid: add Pid abstraction and init_pid_ns helper
  2026-01-29  8:41 ` [PATCH v2 2/3] rust: pid: add Pid abstraction and init_pid_ns helper jongan.kim
@ 2026-01-29 10:32   ` Alice Ryhl
  2026-01-30 11:34     ` heesu0025.kim
  2026-01-29 14:33   ` Gary Guo
  1 sibling, 1 reply; 12+ messages in thread
From: Alice Ryhl @ 2026-01-29 10:32 UTC (permalink / raw)
  To: jongan.kim
  Cc: arve, brauner, cmllamas, gregkh, tkjos, ojeda, boqun.feng, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, dakr, yury.norov,
	vitaly.wool, tamird, viresh.kumar, daniel.almeida, linux-kernel,
	rust-for-linux, heesu0025.kim, ht.hong, jungsu.hwang, kernel-team,
	sanghun.lee, seulgi.lee, sunghoon.kim

On Thu, Jan 29, 2026 at 05:41:18PM +0900, jongan.kim@lge.com wrote:
> From: HeeSu Kim <heesu0025.kim@lge.com>
> 
> Add a new Pid abstraction in rust/kernel/pid.rs that wraps the
> kernel's struct pid and provides safe Rust interfaces for:
> - find_vpid_with_guard: Find a pid by number under RCU protection
> - pid_task_with_guard: Get the task associated with a pid under RCU
>   protection
> 
> Also add init_pid_ns() helper function to pid_namespace.rs to get
> a reference to the init PID namespace.
> 
> These abstractions use lifetime-bounded references tied to RCU guards
> to ensure memory safety when accessing RCU-protected data structures.
> 
> Signed-off-by: HeeSu Kim <heesu0025.kim@lge.com>

This looks really nice, thanks!

> +//! Process identifiers (PIDs).
> +//!
> +//! C header: [`include/linux/pid.h`](srctree/include/linux/pid.h)
> +
> +use crate::{bindings, ffi::c_int, sync::rcu, task::Task, types::Opaque};

Currently we use this formatting for imports:

use crate::{
    bindings,
    ffi::c_int,
    sync::rcu,
    task::Task,
    types::Opaque, //
};

> +/// Wraps the kernel's `struct pid`.
> +///
> +/// This structure represents the Rust abstraction for a C `struct pid`.
> +/// A `Pid` represents a process identifier that can be looked up in different
> +/// PID namespaces.
> +#[repr(transparent)]
> +pub struct Pid {
> +    inner: Opaque<bindings::pid>,
> +}

I would implement Send, Sync, and AlwaysRefCounted for Pid too.

> +            // SAFETY: `find_vpid` returns a valid pointer under RCU protection,
> +            // and `Pid` is `#[repr(transparent)]` over `bindings::pid`.
> +            Some(unsafe { &*(ptr as *const Self) })

It would be nice to extract this cast into a Pid::from_raw().

> +            // SAFETY: `pid_task` returns a valid pointer under RCU protection,
> +            // and `Task` is `#[repr(transparent)]` over `bindings::task_struct`.
> +            Some(unsafe { &*task_ptr.cast() })

I think it would be nice to add a Task::from_raw() to avoid the cast
here.

	Some(unsafe { Task::from_raw(task_ptr) })

> +    pub fn pid_task_with_guard<'a>(&'a self, _rcu_guard: &'a rcu::Guard) -> Option<&'a Task> {
> +    pub fn find_vpid_with_guard<'a>(nr: i32, _rcu_guard: &'a rcu::Guard) -> Option<&'a Self> {

I think we can drop the 'with_guard' suffixes of these.

> +/// Returns a reference to the init PID namespace.
> +///
> +/// This is the root PID namespace that exists throughout the lifetime of the kernel.
> +#[inline]
> +pub fn init_pid_ns() -> &'static PidNamespace {
> +    // SAFETY: `init_pid_ns` is a global static that is valid for the lifetime of the kernel.
> +    unsafe { PidNamespace::from_ptr(core::ptr::addr_of!(bindings::init_pid_ns)) }

Simplifies to:

	PidNamespace::from_ptr(&raw const bindings::init_pid_ns)

Alice

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] rust_binder: handle PID namespace conversion for freeze operation
  2026-01-29  8:41 ` [PATCH v2 3/3] rust_binder: handle PID namespace conversion for freeze operation jongan.kim
@ 2026-01-29 10:35   ` Alice Ryhl
  2026-01-30  5:22     ` heesu0025.kim
  0 siblings, 1 reply; 12+ messages in thread
From: Alice Ryhl @ 2026-01-29 10:35 UTC (permalink / raw)
  To: jongan.kim
  Cc: arve, brauner, cmllamas, gregkh, tkjos, ojeda, boqun.feng, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, dakr, yury.norov,
	vitaly.wool, tamird, viresh.kumar, daniel.almeida, linux-kernel,
	rust-for-linux, heesu0025.kim, ht.hong, jungsu.hwang, kernel-team,
	sanghun.lee, seulgi.lee, sunghoon.kim

On Thu, Jan 29, 2026 at 05:41:19PM +0900, jongan.kim@lge.com 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>

Overall looks good, thanks!

> +/// Convert a PID from the current namespace to the global (init) namespace.
> +fn convert_to_init_ns_tgid(pid: u32) -> Result<i32> {

Let's use the typedef task::Pid (of bindings::pid_t) here, so you can
avoid the `init_ns_pid as _` cast below.

> +    let current = kernel::current!();
> +    let init_ns = init_pid_ns();
> +
> +    if current.active_pid_ns().map(|ns| ns.as_ptr()) == Some(init_ns.as_ptr()) {

I'd like to avoid comparing raw pointers for this. Perhaps we should just provide
an implementation of `==` for PidNamespace that compares the address?

Alice

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] binder: handle PID namespace conversion for freeze operation
  2026-01-29  8:41 ` [PATCH v2 1/3] " jongan.kim
@ 2026-01-29 10:41   ` Alice Ryhl
  2026-01-30  1:54     ` jongan.kim
  0 siblings, 1 reply; 12+ messages in thread
From: Alice Ryhl @ 2026-01-29 10:41 UTC (permalink / raw)
  To: jongan.kim
  Cc: arve, brauner, cmllamas, gregkh, tkjos, ojeda, boqun.feng, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, dakr, yury.norov,
	vitaly.wool, tamird, viresh.kumar, daniel.almeida, linux-kernel,
	rust-for-linux, heesu0025.kim, ht.hong, jungsu.hwang, kernel-team,
	sanghun.lee, seulgi.lee, sunghoon.kim

On Thu, Jan 29, 2026 at 05:41:17PM +0900, jongan.kim@lge.com wrote:
> From: JongAn Kim <jongan.kim@lge.com>
> 
> Currently, when a freeze is attempted from a non-init PID namespace,
> there is a possibility that the wrong process in the init namespace
> may be frozen due to PID collision across namespaces.
> 
> For example, if a container with PID namespace has a process with
> PID 100 (which maps to PID 5000 in init namespace), attempting to
> freeze PID 100 from the container could incorrectly match a different
> process with PID 100 in the init namespace.
> 
> This patch fixes the issue by:
> 1. Converting the caller's PID from their namespace to init namespace
> 2. Matching against binder_proc->pid (which stores init namespace TGID)
> 3. Returning -EINVAL for invalid PIDs and -ESRCH for not-found processes
> 
> This change ensures correct PID handling when binder freeze occurs in
> non-init PID namespace.
> 
> Signed-off-by: JongAn Kim <jongan.kim@lge.com>

> +	rcu_read_lock();
> +	task = pid_task(find_vpid(pid), PIDTYPE_PID);
> +	init_ns_pid = task ? task_tgid_nr_ns(task, &init_pid_ns) : -ESRCH;

You know this is making me think ... here we are obtaining a pointer to
the `struct task_struct`, then we convert it to a pid, and we compare
with the pid of the binder_proc's task.

Why not just outright compare the `struct task_struct` pointers?

Alice

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/3] rust: pid: add Pid abstraction and init_pid_ns helper
  2026-01-29  8:41 ` [PATCH v2 2/3] rust: pid: add Pid abstraction and init_pid_ns helper jongan.kim
  2026-01-29 10:32   ` Alice Ryhl
@ 2026-01-29 14:33   ` Gary Guo
  2026-01-30  4:57     ` heesu0025.kim
  1 sibling, 1 reply; 12+ messages in thread
From: Gary Guo @ 2026-01-29 14:33 UTC (permalink / raw)
  To: jongan.kim, aliceryhl, arve, brauner, cmllamas, gregkh, tkjos,
	ojeda, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
	dakr, yury.norov, vitaly.wool, tamird, viresh.kumar,
	daniel.almeida
  Cc: linux-kernel, rust-for-linux, heesu0025.kim, ht.hong,
	jungsu.hwang, kernel-team, sanghun.lee, seulgi.lee, sunghoon.kim

On Thu Jan 29, 2026 at 8:41 AM GMT, jongan.kim wrote:
> From: HeeSu Kim <heesu0025.kim@lge.com>
>
> Add a new Pid abstraction in rust/kernel/pid.rs that wraps the
> kernel's struct pid and provides safe Rust interfaces for:
> - find_vpid_with_guard: Find a pid by number under RCU protection
> - pid_task_with_guard: Get the task associated with a pid under RCU
>   protection
>
> Also add init_pid_ns() helper function to pid_namespace.rs to get
> a reference to the init PID namespace.
>
> These abstractions use lifetime-bounded references tied to RCU guards
> to ensure memory safety when accessing RCU-protected data structures.
>
> Signed-off-by: HeeSu Kim <heesu0025.kim@lge.com>
> ---
>  rust/kernel/lib.rs           |   1 +
>  rust/kernel/pid.rs           | 100 +++++++++++++++++++++++++++++++++++
>  rust/kernel/pid_namespace.rs |   9 ++++
>  3 files changed, 110 insertions(+)
>  create mode 100644 rust/kernel/pid.rs
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index f812cf120042..60a518d65d0e 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -122,6 +122,7 @@
>  pub mod page;
>  #[cfg(CONFIG_PCI)]
>  pub mod pci;
> +pub mod pid;
>  pub mod pid_namespace;
>  pub mod platform;
>  pub mod prelude;
> diff --git a/rust/kernel/pid.rs b/rust/kernel/pid.rs
> new file mode 100644
> index 000000000000..00989b953c23
> --- /dev/null
> +++ b/rust/kernel/pid.rs
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Process identifiers (PIDs).
> +//!
> +//! C header: [`include/linux/pid.h`](srctree/include/linux/pid.h)
> +
> +use crate::{bindings, ffi::c_int, sync::rcu, task::Task, types::Opaque};
> +
> +/// Wraps the kernel's `struct pid`.
> +///
> +/// This structure represents the Rust abstraction for a C `struct pid`.
> +/// A `Pid` represents a process identifier that can be looked up in different
> +/// PID namespaces.
> +#[repr(transparent)]
> +pub struct Pid {
> +    inner: Opaque<bindings::pid>,
> +}
> +
> +impl Pid {
> +    /// Returns a raw pointer to the inner C struct.
> +    #[inline]
> +    pub fn as_ptr(&self) -> *mut bindings::pid {
> +        self.inner.get()
> +    }
> +
> +    /// Finds a `struct pid` by its pid number within the current task's PID namespace.
> +    ///
> +    /// Returns `None` if no such pid exists.
> +    ///
> +    /// The returned reference is only valid for the duration of the RCU read-side
> +    /// critical section represented by the `rcu::Guard`.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::pid::Pid;
> +    /// use kernel::sync::rcu;
> +    ///
> +    /// let guard = rcu::read_lock();
> +    /// if let Some(pid) = Pid::find_vpid_with_guard(1, &guard) {
> +    ///     pr_info!("Found pid 1\n");
> +    /// }
> +    /// ```
> +    ///
> +    /// Returns `None` for non-existent PIDs:
> +    ///
> +    /// ```
> +    /// use kernel::pid::Pid;
> +    /// use kernel::sync::rcu;
> +    ///
> +    /// let guard = rcu::read_lock();
> +    /// // PID 0 (swapper/idle) is not visible via find_vpid.
> +    /// assert!(Pid::find_vpid_with_guard(0, &guard).is_none());
> +    /// ```
> +    #[inline]
> +    pub fn find_vpid_with_guard<'a>(nr: i32, _rcu_guard: &'a rcu::Guard) -> Option<&'a Self> {
> +        // SAFETY: Called under RCU protection as guaranteed by the Guard reference.
> +        let ptr = unsafe { bindings::find_vpid(nr as c_int) };

This `as c_int` part is not needed.

> +        if ptr.is_null() {
> +            None
> +        } else {
> +            // SAFETY: `find_vpid` returns a valid pointer under RCU protection,
> +            // and `Pid` is `#[repr(transparent)]` over `bindings::pid`.
> +            Some(unsafe { &*(ptr as *const Self) })
> +        }
> +    }
> +
> +    /// Gets the task associated with this PID.
> +    ///
> +    /// Returns `None` if no task is associated with this PID.
> +    ///
> +    /// The returned reference is only valid for the duration of the RCU read-side
> +    /// critical section represented by the `rcu::Guard`.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::pid::Pid;
> +    /// use kernel::sync::rcu;
> +    ///
> +    /// let guard = rcu::read_lock();
> +    /// if let Some(pid) = Pid::find_vpid_with_guard(1, &guard) {
> +    ///     if let Some(task) = pid.pid_task_with_guard(&guard) {
> +    ///         pr_info!("Found task for pid 1\n");
> +    ///     }
> +    /// }
> +    /// ```
> +    #[inline]
> +    pub fn pid_task_with_guard<'a>(&'a self, _rcu_guard: &'a rcu::Guard) -> Option<&'a Task> {
> +        // SAFETY: Called under RCU protection as guaranteed by the Guard reference.
> +        let task_ptr = unsafe { bindings::pid_task(self.as_ptr(), bindings::pid_type_PIDTYPE_PID) };
> +        if task_ptr.is_null() {
> +            None
> +        } else {
> +            // SAFETY: `pid_task` returns a valid pointer under RCU protection,
> +            // and `Task` is `#[repr(transparent)]` over `bindings::task_struct`.
> +            Some(unsafe { &*task_ptr.cast() })
> +        }
> +    }
> +}
> diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
> index 979a9718f153..6029e3e120d0 100644
> --- a/rust/kernel/pid_namespace.rs
> +++ b/rust/kernel/pid_namespace.rs
> @@ -63,3 +63,12 @@ unsafe impl Send for PidNamespace {}
>  // SAFETY: It's OK to access `PidNamespace` through shared references from other threads because
>  // we're either accessing properties that don't change or that are properly synchronised by C code.
>  unsafe impl Sync for PidNamespace {}
> +
> +/// Returns a reference to the init PID namespace.
> +///
> +/// This is the root PID namespace that exists throughout the lifetime of the kernel.
> +#[inline]
> +pub fn init_pid_ns() -> &'static PidNamespace {
> +    // SAFETY: `init_pid_ns` is a global static that is valid for the lifetime of the kernel.
> +    unsafe { PidNamespace::from_ptr(core::ptr::addr_of!(bindings::init_pid_ns)) }
> +}

I would probably put this as assoc fn, so you write `PidNamespace::init_ns()`
instead of `kernel::pid_namespace::init_pid_ns()`.

Best,
Gary

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] binder: handle PID namespace conversion for freeze operation
  2026-01-29 10:41   ` Alice Ryhl
@ 2026-01-30  1:54     ` jongan.kim
  0 siblings, 0 replies; 12+ messages in thread
From: jongan.kim @ 2026-01-30  1:54 UTC (permalink / raw)
  To: aliceryhl
  Cc: a.hindborg, arve, bjorn3_gh, boqun.feng, brauner, cmllamas, dakr,
	daniel.almeida, gary, gregkh, heesu0025.kim, ht.hong, jongan.kim,
	jungsu.hwang, kernel-team, linux-kernel, lossin, ojeda,
	rust-for-linux, sanghun.lee, seulgi.lee, sunghoon.kim, tamird,
	tkjos, tmgross, viresh.kumar, vitaly.wool, yury.norov

Alice Ryhl @ 2026-01-29 10:41 UTC wrote:
> On Thu, Jan 29, 2026 at 05:41:17PM +0900, jongan.kim@lge.com wrote:
> > From: JongAn Kim <jongan.kim@lge.com>
> >
> > Currently, when a freeze is attempted from a non-init PID namespace,
> > there is a possibility that the wrong process in the init namespace
> > may be frozen due to PID collision across namespaces.
> >
> > For example, if a container with PID namespace has a process with
> > PID 100 (which maps to PID 5000 in init namespace), attempting to
> > freeze PID 100 from the container could incorrectly match a different
> > process with PID 100 in the init namespace.
> >
> > This patch fixes the issue by:
> > 1. Converting the caller's PID from their namespace to init namespace
> > 2. Matching against binder_proc->pid (which stores init namespace TGID)
> > 3. Returning -EINVAL for invalid PIDs and -ESRCH for not-found processes
> >
> > This change ensures correct PID handling when binder freeze occurs in
> > non-init PID namespace.
> >
> > Signed-off-by: JongAn Kim <jongan.kim@lge.com>
> 
> > +     rcu_read_lock();
> > +     task = pid_task(find_vpid(pid), PIDTYPE_PID);
> > +     init_ns_pid = task ? task_tgid_nr_ns(task, &init_pid_ns) : -ESRCH;
> 
> You know this is making me think ... here we are obtaining a pointer to
> the `struct task_struct`, then we convert it to a pid, and we compare
> with the pid of the binder_proc's task.
> 
> Why not just outright compare the `struct task_struct` pointers?

Thanks for review and feedback. I hadn't considered that.
I will update patch to compare by using `struct task_struct` pointers.

Thanks. // JongAn, Kim

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/3] rust: pid: add Pid abstraction and init_pid_ns helper
  2026-01-29 14:33   ` Gary Guo
@ 2026-01-30  4:57     ` heesu0025.kim
  0 siblings, 0 replies; 12+ messages in thread
From: heesu0025.kim @ 2026-01-30  4:57 UTC (permalink / raw)
  To: gary
  Cc: a.hindborg, aliceryhl, arve, bjorn3_gh, boqun.feng, brauner,
	cmllamas, dakr, daniel.almeida, gregkh, heesu0025.kim, ht.hong,
	jongan.kim, jungsu.hwang, kernel-team, linux-kernel, lossin,
	ojeda, rust-for-linux, sanghun.lee, seulgi.lee, sunghoon.kim,
	tamird, tkjos, tmgross, viresh.kumar, vitaly.wool, yury.norov

On Thu Jan 29, 2026 at 2:33 PM GMT, Gary Guo wrote:
> On Thu Jan 29, 2026 at 8:41 AM GMT, jongan.kim wrote:
>> From: HeeSu Kim <heesu0025.kim@lge.com>
>>
>> Add a new Pid abstraction in rust/kernel/pid.rs that wraps the
>> kernel's struct pid and provides safe Rust interfaces for:
>> - find_vpid_with_guard: Find a pid by number under RCU protection
>> - pid_task_with_guard: Get the task associated with a pid under RCU
>>   protection
>>
>> Also add init_pid_ns() helper function to pid_namespace.rs to get
>> a reference to the init PID namespace.
>>
>> These abstractions use lifetime-bounded references tied to RCU guards
>> to ensure memory safety when accessing RCU-protected data structures.
>>
>> Signed-off-by: HeeSu Kim <heesu0025.kim@lge.com>
>> ---
>>  rust/kernel/lib.rs           |   1 +
>>  rust/kernel/pid.rs           | 100 +++++++++++++++++++++++++++++++++++
>>  rust/kernel/pid_namespace.rs |   9 ++++
>>  3 files changed, 110 insertions(+)
>>  create mode 100644 rust/kernel/pid.rs
>>
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index f812cf120042..60a518d65d0e 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -122,6 +122,7 @@
>>  pub mod page;
>>  #[cfg(CONFIG_PCI)]
>>  pub mod pci;
>> +pub mod pid;
>>  pub mod pid_namespace;
>>  pub mod platform;
>>  pub mod prelude;
>> diff --git a/rust/kernel/pid.rs b/rust/kernel/pid.rs
>> new file mode 100644
>> index 000000000000..00989b953c23
>> --- /dev/null
>> +++ b/rust/kernel/pid.rs
>> @@ -0,0 +1,100 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Process identifiers (PIDs).
>> +//!
>> +//! C header: [`include/linux/pid.h`](srctree/include/linux/pid.h)
>> +
>> +use crate::{bindings, ffi::c_int, sync::rcu, task::Task, types::Opaque};
>> +
>> +/// Wraps the kernel's `struct pid`.
>> +///
>> +/// This structure represents the Rust abstraction for a C `struct pid`.
>> +/// A `Pid` represents a process identifier that can be looked up in different
>> +/// PID namespaces.
>> +#[repr(transparent)]
>> +pub struct Pid {
>> +    inner: Opaque<bindings::pid>,
>> +}
>> +
>> +impl Pid {
>> +    /// Returns a raw pointer to the inner C struct.
>> +    #[inline]
>> +    pub fn as_ptr(&self) -> *mut bindings::pid {
>> +        self.inner.get()
>> +    }
>> +
>> +    /// Finds a `struct pid` by its pid number within the current task's PID namespace.
>> +    ///
>> +    /// Returns `None` if no such pid exists.
>> +    ///
>> +    /// The returned reference is only valid for the duration of the RCU read-side
>> +    /// critical section represented by the `rcu::Guard`.
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// ```
>> +    /// use kernel::pid::Pid;
>> +    /// use kernel::sync::rcu;
>> +    ///
>> +    /// let guard = rcu::read_lock();
>> +    /// if let Some(pid) = Pid::find_vpid_with_guard(1, &guard) {
>> +    ///     pr_info!("Found pid 1\n");
>> +    /// }
>> +    /// ```
>> +    ///
>> +    /// Returns `None` for non-existent PIDs:
>> +    ///
>> +    /// ```
>> +    /// use kernel::pid::Pid;
>> +    /// use kernel::sync::rcu;
>> +    ///
>> +    /// let guard = rcu::read_lock();
>> +    /// // PID 0 (swapper/idle) is not visible via find_vpid.
>> +    /// assert!(Pid::find_vpid_with_guard(0, &guard).is_none());
>> +    /// ```
>> +    #[inline]
>> +    pub fn find_vpid_with_guard<'a>(nr: i32, _rcu_guard: &'a rcu::Guard) -> Option<&'a Self> {
>> +        // SAFETY: Called under RCU protection as guaranteed by the Guard reference.
>> +        let ptr = unsafe { bindings::find_vpid(nr as c_int) };
>
> This `as c_int` part is not needed.
>
>> +        if ptr.is_null() {
>> +            None
>> +        } else {
>> +            // SAFETY: `find_vpid` returns a valid pointer under RCU protection,
>> +            // and `Pid` is `#[repr(transparent)]` over `bindings::pid`.
>> +            Some(unsafe { &*(ptr as *const Self) })
>> +        }
>> +    }
>> +
>> +    /// Gets the task associated with this PID.
>> +    ///
>> +    /// Returns `None` if no task is associated with this PID.
>> +    ///
>> +    /// The returned reference is only valid for the duration of the RCU read-side
>> +    /// critical section represented by the `rcu::Guard`.
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// ```
>> +    /// use kernel::pid::Pid;
>> +    /// use kernel::sync::rcu;
>> +    ///
>> +    /// let guard = rcu::read_lock();
>> +    /// if let Some(pid) = Pid::find_vpid_with_guard(1, &guard) {
>> +    ///     if let Some(task) = pid.pid_task_with_guard(&guard) {
>> +    ///         pr_info!("Found task for pid 1\n");
>> +    ///     }
>> +    /// }
>> +    /// ```
>> +    #[inline]
>> +    pub fn pid_task_with_guard<'a>(&'a self, _rcu_guard: &'a rcu::Guard) -> Option<&'a Task> {
>> +        // SAFETY: Called under RCU protection as guaranteed by the Guard reference.
>> +        let task_ptr = unsafe { bindings::pid_task(self.as_ptr(), bindings::pid_type_PIDTYPE_PID) };
>> +        if task_ptr.is_null() {
>> +            None
>> +        } else {
>> +            // SAFETY: `pid_task` returns a valid pointer under RCU protection,
>> +            // and `Task` is `#[repr(transparent)]` over `bindings::task_struct`.
>> +            Some(unsafe { &*task_ptr.cast() })
>> +        }
>> +    }
>> +}
>> diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
>> index 979a9718f153..6029e3e120d0 100644
>> --- a/rust/kernel/pid_namespace.rs
>> +++ b/rust/kernel/pid_namespace.rs
>> @@ -63,3 +63,12 @@ unsafe impl Send for PidNamespace {}
>>  // SAFETY: It's OK to access `PidNamespace` through shared references from other threads because
>>  // we're either accessing properties that don't change or that are properly synchronised by C code.
>>  unsafe impl Sync for PidNamespace {}
>> +
>> +/// Returns a reference to the init PID namespace.
>> +///
>> +/// This is the root PID namespace that exists throughout the lifetime of the kernel.
>> +#[inline]
>> +pub fn init_pid_ns() -> &'static PidNamespace {
>> +    // SAFETY: `init_pid_ns` is a global static that is valid for the lifetime of the kernel.
>> +    unsafe { PidNamespace::from_ptr(core::ptr::addr_of!(bindings::init_pid_ns)) }
>> +}
>
> I would probably put this as assoc fn, so you write `PidNamespace::init_ns()`
> instead of `kernel::pid_namespace::init_pid_ns()`.
>
> Best,
> Gary

Thanks for review and feedback.
I will update the patch to address both suggestions in the next revision.

Best Regards,
Heesu Kim

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] rust_binder: handle PID namespace conversion for freeze operation
  2026-01-29 10:35   ` Alice Ryhl
@ 2026-01-30  5:22     ` heesu0025.kim
  0 siblings, 0 replies; 12+ messages in thread
From: heesu0025.kim @ 2026-01-30  5:22 UTC (permalink / raw)
  To: aliceryhl
  Cc: a.hindborg, arve, bjorn3_gh, boqun.feng, brauner, cmllamas, dakr,
	daniel.almeida, gary, gregkh, heesu0025.kim, ht.hong, jongan.kim,
	jungsu.hwang, kernel-team, linux-kernel, lossin, ojeda,
	rust-for-linux, sanghun.lee, seulgi.lee, sunghoon.kim, tamird,
	tkjos, tmgross, viresh.kumar, vitaly.wool, yury.norov

On Thu, Jan 29, 2026 at 10:35:42AM +0000, Alice Ryhl wrote:
> On Thu, Jan 29, 2026 at 05:41:19PM +0900, jongan.kim@lge.com 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>
>
> Overall looks good, thanks!
>
>> +/// Convert a PID from the current namespace to the global (init) namespace.
>> +fn convert_to_init_ns_tgid(pid: u32) -> Result<i32> {
>
> Let's use the typedef task::Pid (of bindings::pid_t) here, so you can
> avoid the `init_ns_pid as _` cast below.
>
>> +    let current = kernel::current!();
>> +    let init_ns = init_pid_ns();
>> +
>> +    if current.active_pid_ns().map(|ns| ns.as_ptr()) == Some(init_ns.as_ptr()) {
>
> I'd like to avoid comparing raw pointers for this. Perhaps we should just provide
> an implementation of `==` for PidNamespace that compares the address?
>
> Alice

Thanks for the review.

I will address both suggestions in the next revision:
- Use task::Pid typedef to avoid the cast
- Implement PartialEq for PidNamespace for direct comparison

Best Regards,
Heesu Kim

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/3] rust: pid: add Pid abstraction and init_pid_ns helper
  2026-01-29 10:32   ` Alice Ryhl
@ 2026-01-30 11:34     ` heesu0025.kim
  0 siblings, 0 replies; 12+ messages in thread
From: heesu0025.kim @ 2026-01-30 11:34 UTC (permalink / raw)
  To: aliceryhl
  Cc: a.hindborg, arve, bjorn3_gh, boqun.feng, brauner, cmllamas, dakr,
	daniel.almeida, gary, gregkh, heesu0025.kim, ht.hong, jongan.kim,
	jungsu.hwang, kernel-team, linux-kernel, lossin, ojeda,
	rust-for-linux, sanghun.lee, seulgi.lee, sunghoon.kim, tamird,
	tkjos, tmgross, viresh.kumar, vitaly.wool, yury.norov

On Thu, Jan 29, 2026 at 10:32:26AM +0000, Alice Ryhl wrote:
> On Thu, Jan 29, 2026 at 05:41:18PM +0900, jongan.kim@lge.com wrote:
>> From: HeeSu Kim <heesu0025.kim@lge.com>
>> 
>> Add a new Pid abstraction in rust/kernel/pid.rs that wraps the
>> kernel's struct pid and provides safe Rust interfaces for:
>> - find_vpid_with_guard: Find a pid by number under RCU protection
>> - pid_task_with_guard: Get the task associated with a pid under RCU
>>   protection
>> 
>> Also add init_pid_ns() helper function to pid_namespace.rs to get
>> a reference to the init PID namespace.
>> 
>> These abstractions use lifetime-bounded references tied to RCU guards
>> to ensure memory safety when accessing RCU-protected data structures.
>> 
>> Signed-off-by: HeeSu Kim <heesu0025.kim@lge.com>
>
> This looks really nice, thanks!
>
>> +//! Process identifiers (PIDs).
>> +//!
>> +//! C header: [`include/linux/pid.h`](srctree/include/linux/pid.h)
>> +
>> +use crate::{bindings, ffi::c_int, sync::rcu, task::Task, types::Opaque};
>
> Currently we use this formatting for imports:
>
> use crate::{
>     bindings,
>     ffi::c_int,
>     sync::rcu,
>     task::Task,
>     types::Opaque, //
> };
>
>> +/// Wraps the kernel's `struct pid`.
>> +///
>> +/// This structure represents the Rust abstraction for a C `struct pid`.
>> +/// A `Pid` represents a process identifier that can be looked up in different
>> +/// PID namespaces.
>> +#[repr(transparent)]
>> +pub struct Pid {
>> +    inner: Opaque<bindings::pid>,
>> +}
>
> I would implement Send, Sync, and AlwaysRefCounted for Pid too.
>
>> +            // SAFETY: `find_vpid` returns a valid pointer under RCU protection,
>> +            // and `Pid` is `#[repr(transparent)]` over `bindings::pid`.
>> +            Some(unsafe { &*(ptr as *const Self) })
>
> It would be nice to extract this cast into a Pid::from_raw().
>
>> +            // SAFETY: `pid_task` returns a valid pointer under RCU protection,
>> +            // and `Task` is `#[repr(transparent)]` over `bindings::task_struct`.
>> +            Some(unsafe { &*task_ptr.cast() })
>
> I think it would be nice to add a Task::from_raw() to avoid the cast
> here.
>
> 	Some(unsafe { Task::from_raw(task_ptr) })
>
>> +    pub fn pid_task_with_guard<'a>(&'a self, _rcu_guard: &'a rcu::Guard) -> Option<&'a Task> {
>> +    pub fn find_vpid_with_guard<'a>(nr: i32, _rcu_guard: &'a rcu::Guard) -> Option<&'a Self> {
>
> I think we can drop the 'with_guard' suffixes of these.
>
>> +/// Returns a reference to the init PID namespace.
>> +///
>> +/// This is the root PID namespace that exists throughout the lifetime of the kernel.
>> +#[inline]
>> +pub fn init_pid_ns() -> &'static PidNamespace {
>> +    // SAFETY: `init_pid_ns` is a global static that is valid for the lifetime of the kernel.
>> +    unsafe { PidNamespace::from_ptr(core::ptr::addr_of!(bindings::init_pid_ns)) }
>
> Simplifies to:
>
> 	PidNamespace::from_ptr(&raw const bindings::init_pid_ns)
>
> Alice

Thanks for the detailed review and feedback.

I agree with your suggestions. While our immediate use case only requires
RCU-protected access, implementing Send, Sync, and AlwaysRefCounted makes
sense from the broader rust/kernel perspective, as these abstractions
will likely be used in various contexts across the kernel.

I will incorporate all suggestions in the next revision.

Best Regards,
Heesu Kim

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-01-30 11:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-29  8:41 [PATCH v2 0/3] binder: handle PID namespace conversion for freeze operation jongan.kim
2026-01-29  8:41 ` [PATCH v2 1/3] " jongan.kim
2026-01-29 10:41   ` Alice Ryhl
2026-01-30  1:54     ` jongan.kim
2026-01-29  8:41 ` [PATCH v2 2/3] rust: pid: add Pid abstraction and init_pid_ns helper jongan.kim
2026-01-29 10:32   ` Alice Ryhl
2026-01-30 11:34     ` heesu0025.kim
2026-01-29 14:33   ` Gary Guo
2026-01-30  4:57     ` heesu0025.kim
2026-01-29  8:41 ` [PATCH v2 3/3] rust_binder: handle PID namespace conversion for freeze operation jongan.kim
2026-01-29 10:35   ` Alice Ryhl
2026-01-30  5:22     ` heesu0025.kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox