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

Changes in v4

1. Patch 1/3
- change subject name more clearly
- comapre task_struct pointers directly instead of PID.

2. Patch 2/3
- Add __rust_helper annotation to rust_helper_get_pid

3. Patch 3/3
- change subject name more clearly
- Use Task pointer comparison instead of PID number comparison (Gary, Alice)
- Remove PidNamespace dependency entirely
- Use &mut KVec parameter to avoid intermediate allocation (Gary)
- Merge context.rs and process.rs changes into single patch

History

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

v2 : https://lore.kernel.org/lkml/20260129084119.32994-1-jongan.kim@lge.com/T/#u
- add two more patches to implement the same logic in Rust binder

v3 : https://lore.kernel.org/lkml/20260203065928.4736-2-jongan.kim@lge.com/

Test
- basic binder freeze test : OK
- binder freeze collision test in seperated namespace : OK

HeeSu Kim (2):
  rust: pid: add Pid abstraction and init_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          |  22 ++++-
 drivers/android/binder/context.rs |  16 +++-
 drivers/android/binder/process.rs |  25 ++++--
 rust/helpers/helpers.c            |   1 +
 rust/helpers/pid.c                |  13 +++
 rust/kernel/lib.rs                |   1 +
 rust/kernel/pid.rs                | 138 ++++++++++++++++++++++++++++++
 rust/kernel/pid_namespace.rs      |  17 ++++
 rust/kernel/task.rs               |  13 +++
 9 files changed, 236 insertions(+), 10 deletions(-)
 create mode 100644 rust/helpers/pid.c
 create mode 100644 rust/kernel/pid.rs

-- 
2.25.1


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

* [PATCH v4 1/3] binder: fix PID namespace collision for freeze operation
  2026-02-06  8:53 [PATCH v4 0/3] binder: handle PID namespace conversion for freeze operation jongan.kim
@ 2026-02-06  8:53 ` jongan.kim
  2026-02-11 10:17   ` jongan.kim
  2026-02-11 10:17   ` jongan.kim
  2026-02-06  8:53 ` [PATCH v4 2/3] rust: pid: add Pid abstraction and init_ns helper jongan.kim
  2026-02-06  8:53 ` [PATCH v4 3/3] rust_binder: fix PID namespace collision for freeze operation jongan.kim
  2 siblings, 2 replies; 12+ messages in thread
From: jongan.kim @ 2026-02-06  8:53 UTC (permalink / raw)
  To: aliceryhl, a.hindborg, arve, bjorn3_gh, boqun.feng, brauner,
	cmllamas, dakr, daniel.almeida, gary, gregkh, tamird, tkjos,
	tmgross, viresh.kumar, vitaly.wool, yury.norov, ojeda, lossin
  Cc: heesu0025.kim, ht.hong, jongan.kim, jungsu.hwang, kernel-team,
	linux-kernel, rust-for-linux, 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. Using find_get_task_by_vpid() to get task_struct from caller's namespace
2. Comparing task_struct pointers directly instead of PID values
3. This ensures we match the exact task regardless of PID namespace

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

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/lkml/aXs5Y3xAFKyZr6nd@google.com/
Signed-off-by: JongAn Kim <jongan.kim@lge.com>
---
v3 -> v4 :
- change subject name more clearly
- comapre task_struct pointers directly instead of PID

v2 -> v3 : change to use task->tgid instead of task_tgid_nr_ns()

 drivers/android/binder.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 535fc881c8da..6d68f98a18db 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5717,13 +5717,18 @@ static int binder_ioctl_get_freezer_info(
 	struct binder_proc *target_proc;
 	bool found = false;
 	__u32 txns_pending;
+	struct task_struct *task;
 
 	info->sync_recv = 0;
 	info->async_recv = 0;
 
+	task = find_get_task_by_vpid(info->pid);
+	if (!task)
+		return -ESRCH;
+
 	mutex_lock(&binder_procs_lock);
 	hlist_for_each_entry(target_proc, &binder_procs, proc_node) {
-		if (target_proc->pid == info->pid) {
+		if (target_proc->tsk == task) {
 			found = true;
 			binder_inner_proc_lock(target_proc);
 			txns_pending = binder_txns_pending_ilocked(target_proc);
@@ -5734,6 +5739,7 @@ static int binder_ioctl_get_freezer_info(
 		}
 	}
 	mutex_unlock(&binder_procs_lock);
+	put_task_struct(task);
 
 	if (!found)
 		return -EINVAL;
@@ -5869,6 +5875,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;
+		struct task_struct *task;
 
 		ret = 0;
 
@@ -5877,14 +5884,21 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			goto err;
 		}
 
+		task = find_get_task_by_vpid(info.pid);
+		if (!task) {
+			ret = -ESRCH;
+			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->tsk == task)
 				target_procs_count++;
 		}
 
 		if (target_procs_count == 0) {
 			mutex_unlock(&binder_procs_lock);
+			put_task_struct(task);
 			ret = -EINVAL;
 			goto err;
 		}
@@ -5895,12 +5909,13 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 
 		if (!target_procs) {
 			mutex_unlock(&binder_procs_lock);
+			put_task_struct(task);
 			ret = -ENOMEM;
 			goto err;
 		}
 
 		hlist_for_each_entry(target_proc, &binder_procs, proc_node) {
-			if (target_proc->pid != info.pid)
+			if (target_proc->tsk != task)
 				continue;
 
 			binder_inner_proc_lock(target_proc);
@@ -5910,6 +5925,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			target_procs[i++] = target_proc;
 		}
 		mutex_unlock(&binder_procs_lock);
+		put_task_struct(task);
 
 		for (i = 0; i < target_procs_count; i++) {
 			if (ret >= 0)
-- 
2.25.1


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

* [PATCH v4 2/3] rust: pid: add Pid abstraction and init_ns helper
  2026-02-06  8:53 [PATCH v4 0/3] binder: handle PID namespace conversion for freeze operation jongan.kim
  2026-02-06  8:53 ` [PATCH v4 1/3] binder: fix PID namespace collision " jongan.kim
@ 2026-02-06  8:53 ` jongan.kim
  2026-02-11 10:42   ` Alice Ryhl
  2026-02-06  8:53 ` [PATCH v4 3/3] rust_binder: fix PID namespace collision for freeze operation jongan.kim
  2 siblings, 1 reply; 12+ messages in thread
From: jongan.kim @ 2026-02-06  8:53 UTC (permalink / raw)
  To: aliceryhl, a.hindborg, arve, bjorn3_gh, boqun.feng, brauner,
	cmllamas, dakr, daniel.almeida, gary, gregkh, tamird, tkjos,
	tmgross, viresh.kumar, vitaly.wool, yury.norov, ojeda, lossin
  Cc: heesu0025.kim, ht.hong, jongan.kim, jungsu.hwang, kernel-team,
	linux-kernel, rust-for-linux, 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: Find a pid by number under RCU protection
- pid_task: Get the task associated with a pid under RCU protection

Also add init_ns() associated function to PidNamespace 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.

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/lkml/aXs3OjlGzQVABAwR@google.com/
Suggested-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/lkml/DG15B78C8IK4.ITL5HKRZ1QKP@garyguo.net/
Signed-off-by: HeeSu Kim <heesu0025.kim@lge.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
---
v3 -> v4:
- Add __rust_helper annotation to rust_helper_get_pid

v2 -> v3:
- Add Send/Sync traits for Pid
- Add AlwaysRefCounted for Pid with rust_helper_get_pid
- Add from_raw() constructor to Pid and Task
- Rename find_vpid_with_guard -> find_vpid
- Rename pid_task_with_guard -> pid_task
- Convert init_pid_ns() to PidNamespace::init_ns()
- Add PartialEq/Eq for PidNamespace
- Add rust/helpers/pid.c for get_pid() wrapper

 rust/helpers/helpers.c       |   1 +
 rust/helpers/pid.c           |  13 ++++
 rust/kernel/lib.rs           |   1 +
 rust/kernel/pid.rs           | 138 +++++++++++++++++++++++++++++++++++
 rust/kernel/pid_namespace.rs |  17 +++++
 rust/kernel/task.rs          |  13 ++++
 6 files changed, 183 insertions(+)
 create mode 100644 rust/helpers/pid.c
 create mode 100644 rust/kernel/pid.rs

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 79c72762ad9c..3138b88df24f 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -38,6 +38,7 @@
 #include "of.c"
 #include "page.c"
 #include "pci.c"
+#include "pid.c"
 #include "pid_namespace.c"
 #include "platform.c"
 #include "poll.c"
diff --git a/rust/helpers/pid.c b/rust/helpers/pid.c
new file mode 100644
index 000000000000..1fb1502da819
--- /dev/null
+++ b/rust/helpers/pid.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/pid.h>
+
+#ifndef __rust_helper
+#define __rust_helper
+#endif
+
+/* Get a reference on a struct pid. */
+__rust_helper struct pid *rust_helper_get_pid(struct pid *pid)
+{
+	return get_pid(pid);
+}
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..253dbf689383
--- /dev/null
+++ b/rust/kernel/pid.rs
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Process identifiers (PIDs).
+//!
+//! C header: [`include/linux/pid.h`](srctree/include/linux/pid.h)
+
+use crate::{
+    bindings,
+    sync::{aref::AlwaysRefCounted, rcu},
+    task::Task,
+    types::Opaque,
+};
+use core::ptr::NonNull;
+
+/// 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>,
+}
+
+// SAFETY: `struct pid` is safely accessible from any thread.
+unsafe impl Send for Pid {}
+
+// SAFETY: `struct pid` is safely accessible from any thread.
+unsafe impl Sync for Pid {}
+
+impl Pid {
+    /// Returns a raw pointer to the inner C struct.
+    #[inline]
+    pub fn as_ptr(&self) -> *mut bindings::pid {
+        self.inner.get()
+    }
+
+    /// Creates a reference to a [`Pid`] from a valid pointer.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+    /// returned [`Pid`] reference.
+    #[inline]
+    pub unsafe fn from_raw<'a>(ptr: *const bindings::pid) -> &'a Self {
+        // SAFETY: The safety requirements guarantee the validity of the dereference, while the
+        // `Pid` type being transparent makes the cast ok.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// 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(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(0, &guard).is_none());
+    /// ```
+    #[inline]
+    pub fn find_vpid<'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) };
+        if ptr.is_null() {
+            None
+        } else {
+            // SAFETY: `find_vpid` returns a valid pointer under RCU protection.
+            Some(unsafe { Self::from_raw(ptr) })
+        }
+    }
+
+    /// 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(1, &guard) {
+    ///     if let Some(task) = pid.pid_task(&guard) {
+    ///         pr_info!("Found task for pid 1\n");
+    ///     }
+    /// }
+    /// ```
+    #[inline]
+    pub fn pid_task<'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.
+            Some(unsafe { Task::from_raw(task_ptr) })
+        }
+    }
+}
+
+// SAFETY: The type invariants guarantee that `Pid` is always refcounted.
+unsafe impl AlwaysRefCounted for Pid {
+    #[inline]
+    fn inc_ref(&self) {
+        // SAFETY: The existence of a shared reference means that the refcount is nonzero.
+        unsafe { bindings::get_pid(self.as_ptr()) };
+    }
+
+    #[inline]
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: The safety requirements guarantee that the refcount is nonzero.
+        unsafe { bindings::put_pid(obj.cast().as_ptr()) }
+    }
+}
diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
index 979a9718f153..fc815945d614 100644
--- a/rust/kernel/pid_namespace.rs
+++ b/rust/kernel/pid_namespace.rs
@@ -38,6 +38,15 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self {
         // `PidNamespace` type being transparent makes the cast ok.
         unsafe { &*ptr.cast() }
     }
+
+    /// 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_ns() -> &'static Self {
+        // SAFETY: `init_pid_ns` is a global static that is valid for the lifetime of the kernel.
+        unsafe { Self::from_ptr(&raw const bindings::init_pid_ns) }
+    }
 }
 
 // SAFETY: Instances of `PidNamespace` are always reference-counted.
@@ -63,3 +72,11 @@ 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 {}
+
+impl PartialEq for PidNamespace {
+    fn eq(&self, other: &Self) -> bool {
+        self.as_ptr() == other.as_ptr()
+    }
+}
+
+impl Eq for PidNamespace {}
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index 49fad6de0674..92a7d27ac3b3 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -204,6 +204,19 @@ pub fn as_ptr(&self) -> *mut bindings::task_struct {
         self.0.get()
     }
 
+    /// Creates a reference to a [`Task`] from a valid pointer.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+    /// returned [`Task`] reference.
+    #[inline]
+    pub unsafe fn from_raw<'a>(ptr: *const bindings::task_struct) -> &'a Self {
+        // SAFETY: The safety requirements guarantee the validity of the dereference, while the
+        // `Task` type being transparent makes the cast ok.
+        unsafe { &*ptr.cast() }
+    }
+
     /// Returns the group leader of the given task.
     pub fn group_leader(&self) -> &Task {
         // SAFETY: The group leader of a task never changes after initialization, so reading this
-- 
2.25.1


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

* [PATCH v4 3/3] rust_binder: fix PID namespace collision for freeze operation
  2026-02-06  8:53 [PATCH v4 0/3] binder: handle PID namespace conversion for freeze operation jongan.kim
  2026-02-06  8:53 ` [PATCH v4 1/3] binder: fix PID namespace collision " jongan.kim
  2026-02-06  8:53 ` [PATCH v4 2/3] rust: pid: add Pid abstraction and init_ns helper jongan.kim
@ 2026-02-06  8:53 ` jongan.kim
  2026-02-06 16:20   ` Gary Guo
  2 siblings, 1 reply; 12+ messages in thread
From: jongan.kim @ 2026-02-06  8:53 UTC (permalink / raw)
  To: aliceryhl, a.hindborg, arve, bjorn3_gh, boqun.feng, brauner,
	cmllamas, dakr, daniel.almeida, gary, gregkh, tamird, tkjos,
	tmgross, viresh.kumar, vitaly.wool, yury.norov, ojeda, lossin
  Cc: heesu0025.kim, ht.hong, jongan.kim, jungsu.hwang, kernel-team,
	linux-kernel, rust-for-linux, 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.

This patch fixes the issue by:
- Add get_task_from_vpid() to translate VPID to Task reference
- Add Context::get_procs_with_task() for Task-based process lookup
  using &mut KVec parameter to avoid intermediate allocations
- Update get_frozen_status() and ioctl_freeze() to use Task comparison

Suggested-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/rust-for-linux/DG5CFX3ML5YL.2FE913F20LNPT@garyguo.net/
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/lkml/aXs5Y3xAFKyZr6nd@google.com/
Signed-off-by: Heesu Kim <heesu0025.kim@lge.com>
---
v3 -> v4
- change subject name more clearly
- Use Task pointer comparison instead of PID number comparison
- Remove PidNamespace dependency entirely
- Use &mut KVec parameter to avoid intermediate allocation
- Merge context.rs and process.rs changes into single patch
 
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/context.rs | 16 +++++++++++++++-
 drivers/android/binder/process.rs | 25 +++++++++++++++++++------
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/drivers/android/binder/context.rs b/drivers/android/binder/context.rs
index 3d135ec03ca7..1fc779e4d9ce 100644
--- a/drivers/android/binder/context.rs
+++ b/drivers/android/binder/context.rs
@@ -9,7 +9,7 @@
     security,
     str::{CStr, CString},
     sync::{Arc, Mutex},
-    task::Kuid,
+    task::{Kuid, Task},
 };
 
 use crate::{error::BinderError, node::NodeRef, process::Process};
@@ -177,4 +177,18 @@ pub(crate) fn get_procs_with_pid(&self, pid: i32) -> Result<KVec<Arc<Process>>>
         }
         Ok(backing)
     }
+
+    pub(crate) fn get_procs_with_task(
+        &self,
+        target: &Task,
+        out: &mut KVec<Arc<Process>>,
+    ) -> Result {
+        let lock = self.manager.lock();
+        for proc in &lock.all_procs {
+            if core::ptr::eq(&*proc.task, target) {
+                out.push(Arc::from(proc), GFP_KERNEL)?;
+            }
+        }
+        Ok(())
+    }
 }
diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 132055b4790f..58e816f8873f 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -22,6 +22,7 @@
     id_pool::IdPool,
     list::{List, ListArc, ListArcField, ListLinks},
     mm,
+    pid::Pid,
     prelude::*,
     rbtree::{self, RBTree, RBTreeNode, RBTreeNodeReservation},
     seq_file::SeqFile,
@@ -29,9 +30,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 +1499,29 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
     }
 }
 
+/// Get Task reference from VPID with refcount increment.
+fn get_task_from_vpid(pid: PidT) -> Result<ARef<Task>> {
+    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(ARef::from(task))
+}
+
 fn get_frozen_status(data: UserSlice) -> Result {
     let (mut reader, mut writer) = data.reader_writer();
 
     let mut info = reader.read::<BinderFrozenStatusInfo>()?;
+
+    let target_task = get_task_from_vpid(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 core::ptr::eq(&*proc.task, &*target_task) {
                 found = true;
                 let inner = proc.inner.lock();
                 let txns_pending = inner.txns_pending_locked();
@@ -1530,15 +1543,15 @@ fn get_frozen_status(data: UserSlice) -> Result {
 fn ioctl_freeze(reader: &mut UserSliceReader) -> Result {
     let info = reader.read::<BinderFreezeInfo>()?;
 
+    let target_task = get_task_from_vpid(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)? {
-            procs.push(proc, GFP_KERNEL)?;
-        }
+        ctx.get_procs_with_task(&target_task, &mut procs)?;
     }
 
     for proc in procs {
-- 
2.25.1


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

* Re: [PATCH v4 3/3] rust_binder: fix PID namespace collision for freeze operation
  2026-02-06  8:53 ` [PATCH v4 3/3] rust_binder: fix PID namespace collision for freeze operation jongan.kim
@ 2026-02-06 16:20   ` Gary Guo
  2026-02-09  5:21     ` heesu0025.kim
  0 siblings, 1 reply; 12+ messages in thread
From: Gary Guo @ 2026-02-06 16:20 UTC (permalink / raw)
  To: jongan.kim, aliceryhl, a.hindborg, arve, bjorn3_gh, boqun.feng,
	brauner, cmllamas, dakr, daniel.almeida, gary, gregkh, tamird,
	tkjos, tmgross, viresh.kumar, vitaly.wool, yury.norov, ojeda,
	lossin
  Cc: heesu0025.kim, ht.hong, jungsu.hwang, kernel-team, linux-kernel,
	rust-for-linux, sanghun.lee, seulgi.lee, sunghoon.kim

On Fri Feb 6, 2026 at 8:53 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.
>
> This patch fixes the issue by:
> - Add get_task_from_vpid() to translate VPID to Task reference
> - Add Context::get_procs_with_task() for Task-based process lookup
>   using &mut KVec parameter to avoid intermediate allocations
> - Update get_frozen_status() and ioctl_freeze() to use Task comparison
>
> Suggested-by: Gary Guo <gary@garyguo.net>
> Link: https://lore.kernel.org/rust-for-linux/DG5CFX3ML5YL.2FE913F20LNPT@garyguo.net/
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Link: https://lore.kernel.org/lkml/aXs5Y3xAFKyZr6nd@google.com/
> Signed-off-by: Heesu Kim <heesu0025.kim@lge.com>
> ---
> v3 -> v4
> - change subject name more clearly
> - Use Task pointer comparison instead of PID number comparison
> - Remove PidNamespace dependency entirely
> - Use &mut KVec parameter to avoid intermediate allocation
> - Merge context.rs and process.rs changes into single patch
>  
> 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/context.rs | 16 +++++++++++++++-
>  drivers/android/binder/process.rs | 25 +++++++++++++++++++------
>  2 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/android/binder/context.rs b/drivers/android/binder/context.rs
> index 3d135ec03ca7..1fc779e4d9ce 100644
> --- a/drivers/android/binder/context.rs
> +++ b/drivers/android/binder/context.rs
> @@ -9,7 +9,7 @@
>      security,
>      str::{CStr, CString},
>      sync::{Arc, Mutex},
> -    task::Kuid,
> +    task::{Kuid, Task},
>  };
>  
>  use crate::{error::BinderError, node::NodeRef, process::Process};
> @@ -177,4 +177,18 @@ pub(crate) fn get_procs_with_pid(&self, pid: i32) -> Result<KVec<Arc<Process>>>
>          }
>          Ok(backing)
>      }
> +
> +    pub(crate) fn get_procs_with_task(
> +        &self,
> +        target: &Task,
> +        out: &mut KVec<Arc<Process>>,
> +    ) -> Result {
> +        let lock = self.manager.lock();
> +        for proc in &lock.all_procs {
> +            if core::ptr::eq(&*proc.task, target) {

I think we should just as a `PartialEq` and `Eq` impl to `Task`.

> +                out.push(Arc::from(proc), GFP_KERNEL)?;
> +            }
> +        }
> +        Ok(())
> +    }
>  }
> diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
> index 132055b4790f..58e816f8873f 100644
> --- a/drivers/android/binder/process.rs
> +++ b/drivers/android/binder/process.rs
> @@ -22,6 +22,7 @@
>      id_pool::IdPool,
>      list::{List, ListArc, ListArcField, ListLinks},
>      mm,
> +    pid::Pid,
>      prelude::*,
>      rbtree::{self, RBTree, RBTreeNode, RBTreeNodeReservation},
>      seq_file::SeqFile,
> @@ -29,9 +30,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 +1499,29 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
>      }
>  }
>  
> +/// Get Task reference from VPID with refcount increment.
> +fn get_task_from_vpid(pid: PidT) -> Result<ARef<Task>> {
> +    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(ARef::from(task))
> +}
> +
>  fn get_frozen_status(data: UserSlice) -> Result {
>      let (mut reader, mut writer) = data.reader_writer();
>  
>      let mut info = reader.read::<BinderFrozenStatusInfo>()?;
> +
> +    let target_task = get_task_from_vpid(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 core::ptr::eq(&*proc.task, &*target_task) {

Same here, could use `==` if you have `PartialEq` impl on task.

Best,
Gary

>                  found = true;
>                  let inner = proc.inner.lock();
>                  let txns_pending = inner.txns_pending_locked();
> @@ -1530,15 +1543,15 @@ fn get_frozen_status(data: UserSlice) -> Result {
>  fn ioctl_freeze(reader: &mut UserSliceReader) -> Result {
>      let info = reader.read::<BinderFreezeInfo>()?;
>  
> +    let target_task = get_task_from_vpid(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)? {
> -            procs.push(proc, GFP_KERNEL)?;
> -        }
> +        ctx.get_procs_with_task(&target_task, &mut procs)?;
>      }
>  
>      for proc in procs {


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

* Re: [PATCH v4 3/3] rust_binder: fix PID namespace collision for freeze operation
  2026-02-06 16:20   ` Gary Guo
@ 2026-02-09  5:21     ` heesu0025.kim
  0 siblings, 0 replies; 12+ messages in thread
From: heesu0025.kim @ 2026-02-09  5:21 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

Hi Gary,

Thank you for the review and suggestion.

On Fri, Feb 7, 2026 at 4:20 PM GMT, Gary Guo wrote:
> On Fri Feb 6, 2026 at 8:53 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.
> >
> > This patch fixes the issue by:
> > - Add get_task_from_vpid() to translate VPID to Task reference
> > - Add Context::get_procs_with_task() for Task-based process lookup
> >   using &mut KVec parameter to avoid intermediate allocations
> > - Update get_frozen_status() and ioctl_freeze() to use Task comparison
> >
> > Suggested-by: Gary Guo <gary@garyguo.net>
> > Link: https://lore.kernel.org/rust-for-linux/DG5CFX3ML5YL.2FE913F20LNPT@garyguo.net/
> > Suggested-by: Alice Ryhl <aliceryhl@google.com>
> > Link: https://lore.kernel.org/lkml/aXs5Y3xAFKyZr6nd@google.com/
> > Signed-off-by: Heesu Kim <heesu0025.kim@lge.com>
> > ---
> > v3 -> v4
> > - change subject name more clearly
> > - Use Task pointer comparison instead of PID number comparison
> > - Remove PidNamespace dependency entirely
> > - Use &mut KVec parameter to avoid intermediate allocation
> > - Merge context.rs and process.rs changes into single patch
> >
> > 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/context.rs | 16 +++++++++++++++-
> >  drivers/android/binder/process.rs | 25 +++++++++++++++++++------
> >  2 files changed, 34 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/android/binder/context.rs b/drivers/android/binder/context.rs
> > index 3d135ec03ca7..1fc779e4d9ce 100644
> > --- a/drivers/android/binder/context.rs
> > +++ b/drivers/android/binder/context.rs
> > @@ -9,7 +9,7 @@
> >      security,
> >      str::{CStr, CString},
> >      sync::{Arc, Mutex},
> > -    task::Kuid,
> > +    task::{Kuid, Task},
> >  };
> >
> >  use crate::{error::BinderError, node::NodeRef, process::Process};
> > @@ -177,4 +177,18 @@ pub(crate) fn get_procs_with_pid(&self, pid: i32) -> Result<KVec<Arc<Process>>>
> >          }
> >          Ok(backing)
> >      }
> > +
> > +    pub(crate) fn get_procs_with_task(
> > +        &self,
> > +        target: &Task,
> > +        out: &mut KVec<Arc<Process>>,
> > +    ) -> Result {
> > +        let lock = self.manager.lock();
> > +        for proc in &lock.all_procs {
> > +            if core::ptr::eq(&*proc.task, target) {
>
> I think we should just add a `PartialEq` and `Eq` impl to `Task`.

I agree, this would make the comparisons much cleaner. I'm planning to
implement this as follows:

In rust/kernel/task.rs, add:
  impl PartialEq for Task {
      fn eq(&self, other: &Self) -> bool {
          self.as_ptr() == other.as_ptr()
      }
  }

  impl Eq for Task {}

This follows the same pattern we used for PidNamespace and allows the
binder code to use natural comparisons:
  if *proc.task == *target {
instead of:
  if core::ptr::eq(&*proc.task, target) {

> > +                out.push(Arc::from(proc), GFP_KERNEL)?;
> > +            }
> > +        }
> > +        Ok(())
> > +    }
> >  }
> > diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
> > index 132055b4790f..58e816f8873f 100644
> > --- a/drivers/android/binder/process.rs
> > +++ b/drivers/android/binder/process.rs
> > @@ -22,6 +22,7 @@
> >      id_pool::IdPool,
> >      list::{List, ListArc, ListArcField, ListLinks},
> >      mm,
> > +    pid::Pid,
> >      prelude::*,
> >      rbtree::{self, RBTree, RBTreeNode, RBTreeNodeReservation},
> >      seq_file::SeqFile,
> > @@ -29,9 +30,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 +1499,29 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
> >      }
> >  }
> >
> > +/// Get Task reference from VPID with refcount increment.
> > +fn get_task_from_vpid(pid: PidT) -> Result<ARef<Task>> {
> > +    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(ARef::from(task))
> > +}
> > +
> >  fn get_frozen_status(data: UserSlice) -> Result {
> >      let (mut reader, mut writer) = data.reader_writer();
> >
> >      let mut info = reader.read::<BinderFrozenStatusInfo>()?;
> > +
> > +    let target_task = get_task_from_vpid(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 core::ptr::eq(&*proc.task, &*target_task) {
>
> Same here, could use `==` if you have `PartialEq` impl on task.

For the patch structure, I'm planning to split v5 into:
  2/4: rust: pid: add Pid abstraction (unchanged, keeps your Reviewed-by)
  3/4: rust: task: add PartialEq and Eq for Task
  4/4: rust_binder: use VPID translation for freeze operation

This way your existing review on 2/4 is preserved, and the Task equality
implementation can be reviewed independently.

Does this approach look correct?

Best regards,
Heesu

> >                  found = true;
> >                  let inner = proc.inner.lock();
> >                  let txns_pending = inner.txns_pending_locked();
> > @@ -1530,15 +1543,15 @@ fn get_frozen_status(data: UserSlice) -> Result {
> >  fn ioctl_freeze(reader: &mut UserSliceReader) -> Result {
> >      let info = reader.read::<BinderFreezeInfo>()?;
> >
> > +    let target_task = get_task_from_vpid(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)? {
> > -            procs.push(proc, GFP_KERNEL)?;
> > -        }
> > +        ctx.get_procs_with_task(&target_task, &mut procs)?;
> >      }
> >
> >      for proc in procs {
>
> Best,
> Gary

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

* Re: [PATCH v4 1/3] binder: fix PID namespace collision for freeze operation
  2026-02-06  8:53 ` [PATCH v4 1/3] binder: fix PID namespace collision " jongan.kim
@ 2026-02-11 10:17   ` jongan.kim
  2026-02-11 10:17   ` jongan.kim
  1 sibling, 0 replies; 12+ messages in thread
From: jongan.kim @ 2026-02-11 10:17 UTC (permalink / raw)
  To: jongan.kim, aliceryhl, arve, brauner, cmllamas, tkjos
  Cc: a.hindborg, bjorn3_gh, boqun.feng, dakr, daniel.almeida, gary,
	gregkh, heesu0025.kim, ht.hong, jungsu.hwang, kernel-team,
	linux-kernel, lossin, ojeda, rust-for-linux, sanghun.lee,
	seulgi.lee, sunghoon.kim, tamird, tmgross, viresh.kumar,
	vitaly.wool, yury.norov

On Fri, Feb 06 2026 at 17:53:34PM +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. Using find_get_task_by_vpid() to get task_struct from caller's namespace
> 2. Comparing task_struct pointers directly instead of PID values
> 3. This ensures we match the exact task regardless of PID namespace
> 
> This change ensures correct PID handling when binder freeze occurs in
> non-init PID namespace.
> 
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Link: https://lore.kernel.org/lkml/aXs5Y3xAFKyZr6nd@google.com/
> Signed-off-by: JongAn Kim <jongan.kim@lge.com>
> ---
> v3 -> v4 :
> - change subject name more clearly
> - comapre task_struct pointers directly instead of PID
> 
> v2 -> v3 : change to use task->tgid instead of task_tgid_nr_ns()
> 
>  drivers/android/binder.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 535fc881c8da..6d68f98a18db 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5717,13 +5717,18 @@ static int binder_ioctl_get_freezer_info(
>  	struct binder_proc *target_proc;
>  	bool found = false;
>  	__u32 txns_pending;
> +	struct task_struct *task;
>  
>  	info->sync_recv = 0;
>  	info->async_recv = 0;
>  
> +	task = find_get_task_by_vpid(info->pid);
> +	if (!task)
> +		return -ESRCH;
> +
>  	mutex_lock(&binder_procs_lock);
>  	hlist_for_each_entry(target_proc, &binder_procs, proc_node) {
> -		if (target_proc->pid == info->pid) {
> +		if (target_proc->tsk == task) {
>  			found = true;
>  			binder_inner_proc_lock(target_proc);
>  			txns_pending = binder_txns_pending_ilocked(target_proc);
> @@ -5734,6 +5739,7 @@ static int binder_ioctl_get_freezer_info(
>  		}
>  	}
>  	mutex_unlock(&binder_procs_lock);
> +	put_task_struct(task);
>  
>  	if (!found)
>  		return -EINVAL;
> @@ -5869,6 +5875,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;
> +		struct task_struct *task;
>  
>  		ret = 0;
>  
> @@ -5877,14 +5884,21 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  			goto err;
>  		}
>  
> +		task = find_get_task_by_vpid(info.pid);
> +		if (!task) {
> +			ret = -ESRCH;
> +			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->tsk == task)
>  				target_procs_count++;
>  		}
>  
>  		if (target_procs_count == 0) {
>  			mutex_unlock(&binder_procs_lock);
> +			put_task_struct(task);
>  			ret = -EINVAL;
>  			goto err;
>  		}
> @@ -5895,12 +5909,13 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  
>  		if (!target_procs) {
>  			mutex_unlock(&binder_procs_lock);
> +			put_task_struct(task);
>  			ret = -ENOMEM;
>  			goto err;
>  		}
>  
>  		hlist_for_each_entry(target_proc, &binder_procs, proc_node) {
> -			if (target_proc->pid != info.pid)
> +			if (target_proc->tsk != task)
>  				continue;
>  
>  			binder_inner_proc_lock(target_proc);
> @@ -5910,6 +5925,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  			target_procs[i++] = target_proc;
>  		}
>  		mutex_unlock(&binder_procs_lock);
> +		put_task_struct(task);
>  
>  		for (i = 0; i < target_procs_count; i++) {
>  			if (ret >= 0)gg

Dear Alice and Android Driver Maintainers,

I would appreciate it if you could review patch v4.

Thank you for your time and effort.

Best regards,
Jong An Kim

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

* Re: [PATCH v4 1/3] binder: fix PID namespace collision for freeze operation
  2026-02-06  8:53 ` [PATCH v4 1/3] binder: fix PID namespace collision " jongan.kim
  2026-02-11 10:17   ` jongan.kim
@ 2026-02-11 10:17   ` jongan.kim
  2026-02-11 11:13     ` Greg KH
  1 sibling, 1 reply; 12+ messages in thread
From: jongan.kim @ 2026-02-11 10:17 UTC (permalink / raw)
  To: jongan.kim, aliceryhl, arve, brauner, cmllamas, tkjos
  Cc: a.hindborg, bjorn3_gh, boqun.feng, dakr, daniel.almeida, gary,
	gregkh, heesu0025.kim, ht.hong, jungsu.hwang, kernel-team,
	linux-kernel, lossin, ojeda, rust-for-linux, sanghun.lee,
	seulgi.lee, sunghoon.kim, tamird, tmgross, viresh.kumar,
	vitaly.wool, yury.norov

On Fri, Feb 06 2026 at 17:53:34PM +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. Using find_get_task_by_vpid() to get task_struct from caller's namespace
> 2. Comparing task_struct pointers directly instead of PID values
> 3. This ensures we match the exact task regardless of PID namespace
> 
> This change ensures correct PID handling when binder freeze occurs in
> non-init PID namespace.
> 
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Link: https://lore.kernel.org/lkml/aXs5Y3xAFKyZr6nd@google.com/
> Signed-off-by: JongAn Kim <jongan.kim@lge.com>
> ---
> v3 -> v4 :
> - change subject name more clearly
> - comapre task_struct pointers directly instead of PID
> 
> v2 -> v3 : change to use task->tgid instead of task_tgid_nr_ns()
> 
>  drivers/android/binder.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 535fc881c8da..6d68f98a18db 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5717,13 +5717,18 @@ static int binder_ioctl_get_freezer_info(
>  	struct binder_proc *target_proc;
>  	bool found = false;
>  	__u32 txns_pending;
> +	struct task_struct *task;
>  
>  	info->sync_recv = 0;
>  	info->async_recv = 0;
>  
> +	task = find_get_task_by_vpid(info->pid);
> +	if (!task)
> +		return -ESRCH;
> +
>  	mutex_lock(&binder_procs_lock);
>  	hlist_for_each_entry(target_proc, &binder_procs, proc_node) {
> -		if (target_proc->pid == info->pid) {
> +		if (target_proc->tsk == task) {
>  			found = true;
>  			binder_inner_proc_lock(target_proc);
>  			txns_pending = binder_txns_pending_ilocked(target_proc);
> @@ -5734,6 +5739,7 @@ static int binder_ioctl_get_freezer_info(
>  		}
>  	}
>  	mutex_unlock(&binder_procs_lock);
> +	put_task_struct(task);
>  
>  	if (!found)
>  		return -EINVAL;
> @@ -5869,6 +5875,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;
> +		struct task_struct *task;
>  
>  		ret = 0;
>  
> @@ -5877,14 +5884,21 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  			goto err;
>  		}
>  
> +		task = find_get_task_by_vpid(info.pid);
> +		if (!task) {
> +			ret = -ESRCH;
> +			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->tsk == task)
>  				target_procs_count++;
>  		}
>  
>  		if (target_procs_count == 0) {
>  			mutex_unlock(&binder_procs_lock);
> +			put_task_struct(task);
>  			ret = -EINVAL;
>  			goto err;
>  		}
> @@ -5895,12 +5909,13 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  
>  		if (!target_procs) {
>  			mutex_unlock(&binder_procs_lock);
> +			put_task_struct(task);
>  			ret = -ENOMEM;
>  			goto err;
>  		}
>  
>  		hlist_for_each_entry(target_proc, &binder_procs, proc_node) {
> -			if (target_proc->pid != info.pid)
> +			if (target_proc->tsk != task)
>  				continue;
>  
>  			binder_inner_proc_lock(target_proc);
> @@ -5910,6 +5925,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  			target_procs[i++] = target_proc;
>  		}
>  		mutex_unlock(&binder_procs_lock);
> +		put_task_struct(task);
>  
>  		for (i = 0; i < target_procs_count; i++) {
>  			if (ret >= 0)gg

Dear Alice and Android Driver Maintainers,

I would appreciate it if you could review patch v4.

Thank you for your time and effort.

Best regards,
Jong An Kim

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

* Re: [PATCH v4 2/3] rust: pid: add Pid abstraction and init_ns helper
  2026-02-06  8:53 ` [PATCH v4 2/3] rust: pid: add Pid abstraction and init_ns helper jongan.kim
@ 2026-02-11 10:42   ` Alice Ryhl
  2026-02-13  5:15     ` jongan.kim
  0 siblings, 1 reply; 12+ messages in thread
From: Alice Ryhl @ 2026-02-11 10:42 UTC (permalink / raw)
  To: jongan.kim
  Cc: a.hindborg, arve, bjorn3_gh, boqun.feng, brauner, cmllamas, dakr,
	daniel.almeida, gary, gregkh, tamird, tkjos, tmgross,
	viresh.kumar, vitaly.wool, yury.norov, ojeda, lossin,
	heesu0025.kim, ht.hong, jungsu.hwang, kernel-team, linux-kernel,
	rust-for-linux, sanghun.lee, seulgi.lee, sunghoon.kim

On Fri, Feb 06, 2026 at 05:53:35PM +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: Find a pid by number under RCU protection
> - pid_task: Get the task associated with a pid under RCU protection
> 
> Also add init_ns() associated function to PidNamespace 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.
> 
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Link: https://lore.kernel.org/lkml/aXs3OjlGzQVABAwR@google.com/
> Suggested-by: Gary Guo <gary@garyguo.net>
> Link: https://lore.kernel.org/lkml/DG15B78C8IK4.ITL5HKRZ1QKP@garyguo.net/
> Signed-off-by: HeeSu Kim <heesu0025.kim@lge.com>
> Reviewed-by: Gary Guo <gary@garyguo.net>

> +#ifndef __rust_helper
> +#define __rust_helper
> +#endif

This should not be included. __rust_helper is always defined.

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

This should use the typedef of pid_t (in task.rs) instead of i32.

> diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
> index 979a9718f153..fc815945d614 100644
> --- a/rust/kernel/pid_namespace.rs
> +++ b/rust/kernel/pid_namespace.rs
> @@ -38,6 +38,15 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self {
>          // `PidNamespace` type being transparent makes the cast ok.
>          unsafe { &*ptr.cast() }
>      }
> +
> +    /// 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_ns() -> &'static Self {
> +        // SAFETY: `init_pid_ns` is a global static that is valid for the lifetime of the kernel.
> +        unsafe { Self::from_ptr(&raw const bindings::init_pid_ns) }
> +    }

This is no longer used anywhere.

>  }
>  
>  // SAFETY: Instances of `PidNamespace` are always reference-counted.
> @@ -63,3 +72,11 @@ 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 {}
> +
> +impl PartialEq for PidNamespace {
> +    fn eq(&self, other: &Self) -> bool {
> +        self.as_ptr() == other.as_ptr()
> +    }
> +}
> +impl Eq for PidNamespace {}

This is not used anywhere either.

Alice

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

* Re: [PATCH v4 1/3] binder: fix PID namespace collision for freeze operation
  2026-02-11 10:17   ` jongan.kim
@ 2026-02-11 11:13     ` Greg KH
  2026-02-12  1:05       ` jongan.kim
  0 siblings, 1 reply; 12+ messages in thread
From: Greg KH @ 2026-02-11 11:13 UTC (permalink / raw)
  To: jongan.kim
  Cc: aliceryhl, arve, brauner, cmllamas, tkjos, a.hindborg, bjorn3_gh,
	boqun.feng, dakr, daniel.almeida, gary, heesu0025.kim, ht.hong,
	jungsu.hwang, kernel-team, linux-kernel, lossin, ojeda,
	rust-for-linux, sanghun.lee, seulgi.lee, sunghoon.kim, tamird,
	tmgross, viresh.kumar, vitaly.wool, yury.norov

On Wed, Feb 11, 2026 at 07:17:37PM +0900, jongan.kim@lge.com wrote:
> On Fri, Feb 06 2026 at 17:53:34PM +0900, jongan.kim@lge.com wrote:

<snip>

> Dear Alice and Android Driver Maintainers,
> 
> I would appreciate it if you could review patch v4.

It has been less than 1 week since you sent this, and we are in the
middle of the merge window when nothing can be accepted at all until
after -rc1 is out.

Please relax, there is no rush here.  If you wish to help out, please
help in reviewing other patches that have been submitted for review, as
that will allow your changes to go up the stack.

thanks,

greg k-h

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

* Re: [PATCH v4 1/3] binder: fix PID namespace collision for freeze operation
  2026-02-11 11:13     ` Greg KH
@ 2026-02-12  1:05       ` jongan.kim
  0 siblings, 0 replies; 12+ messages in thread
From: jongan.kim @ 2026-02-12  1:05 UTC (permalink / raw)
  To: gregkh
  Cc: a.hindborg, aliceryhl, arve, bjorn3_gh, boqun.feng, brauner,
	cmllamas, dakr, daniel.almeida, gary, 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

On Wed, Feb 11 2026 at 12:13:38 +0100, Greg KH wrote:
> On Wed, Feb 11, 2026 at 07:17:37PM +0900, jongan.kim@lge.com wrote:
> > On Fri, Feb 06 2026 at 17:53:34PM +0900, jongan.kim@lge.com wrote:
> 
> <snip>
> 
> > Dear Alice and Android Driver Maintainers,
> > 
> > I would appreciate it if you could review patch v4.
> 
> It has been less than 1 week since you sent this, and we are in the
> middle of the merge window when nothing can be accepted at all until
> after -rc1 is out.
> 
> Please relax, there is no rush here.  If you wish to help out, please
> help in reviewing other patches that have been submitted for review, as
> that will allow your changes to go up the stack.
> 
> thanks,
> 
> greg k-h
> 

Thank you for the guidance.
I understand and will wait for feedback.

Thnks for your time.

Best regards,
Jong An Kim


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

* Re: [PATCH v4 2/3] rust: pid: add Pid abstraction and init_ns helper
  2026-02-11 10:42   ` Alice Ryhl
@ 2026-02-13  5:15     ` jongan.kim
  0 siblings, 0 replies; 12+ messages in thread
From: jongan.kim @ 2026-02-13  5:15 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 Wed, Feb 11, 2026 at 10:42:30 +0000, Alice Ryhl wrote:
> On Fri, Feb 06, 2026 at 05:53:35PM +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: Find a pid by number under RCU protection
> > - pid_task: Get the task associated with a pid under RCU protection
> > 
> > Also add init_ns() associated function to PidNamespace 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.
> > 
> > Suggested-by: Alice Ryhl <aliceryhl@google.com>
> > Link: https://lore.kernel.org/lkml/aXs3OjlGzQVABAwR@google.com/
> > Suggested-by: Gary Guo <gary@garyguo.net>
> > Link: https://lore.kernel.org/lkml/DG15B78C8IK4.ITL5HKRZ1QKP@garyguo.net/
> > Signed-off-by: HeeSu Kim <heesu0025.kim@lge.com>
> > Reviewed-by: Gary Guo <gary@garyguo.net>
> 
> > +#ifndef __rust_helper
> > +#define __rust_helper
> > +#endif
> 
> This should not be included. __rust_helper is always defined.
> 
> > +    pub fn find_vpid<'a>(nr: i32, _rcu_guard: &'a rcu::Guard) -> Option<&'a Self> {
> 
> This should use the typedef of pid_t (in task.rs) instead of i32.
> 
> > diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
> > index 979a9718f153..fc815945d614 100644
> > --- a/rust/kernel/pid_namespace.rs
> > +++ b/rust/kernel/pid_namespace.rs
> > @@ -38,6 +38,15 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self {
> >          // `PidNamespace` type being transparent makes the cast ok.
> >          unsafe { &*ptr.cast() }
> >      }
> > +
> > +    /// 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_ns() -> &'static Self {
> > +        // SAFETY: `init_pid_ns` is a global static that is valid for the lifetime of the kernel.
> > +        unsafe { Self::from_ptr(&raw const bindings::init_pid_ns) }
> > +    }
> 
> This is no longer used anywhere.
> 
> >  }
> >  
> >  // SAFETY: Instances of `PidNamespace` are always reference-counted.
> > @@ -63,3 +72,11 @@ 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 {}
> > +
> > +impl PartialEq for PidNamespace {
> > +    fn eq(&self, other: &Self) -> bool {
> > +        self.as_ptr() == other.as_ptr()
> > +    }
> > +}
> > +impl Eq for PidNamespace {}
> 
> This is not used anywhere either.
> 
> Alice

Thank you for feedback.
We will submit the next patch version reflecting your review.
- remove __rust_helper define.
- use pid_t instead of i32
- remove unused implementations in pid_namespace.rs

Best Regards,
Jong An, Kim.
 

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

end of thread, other threads:[~2026-02-13  5:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-06  8:53 [PATCH v4 0/3] binder: handle PID namespace conversion for freeze operation jongan.kim
2026-02-06  8:53 ` [PATCH v4 1/3] binder: fix PID namespace collision " jongan.kim
2026-02-11 10:17   ` jongan.kim
2026-02-11 10:17   ` jongan.kim
2026-02-11 11:13     ` Greg KH
2026-02-12  1:05       ` jongan.kim
2026-02-06  8:53 ` [PATCH v4 2/3] rust: pid: add Pid abstraction and init_ns helper jongan.kim
2026-02-11 10:42   ` Alice Ryhl
2026-02-13  5:15     ` jongan.kim
2026-02-06  8:53 ` [PATCH v4 3/3] rust_binder: fix PID namespace collision for freeze operation jongan.kim
2026-02-06 16:20   ` Gary Guo
2026-02-09  5:21     ` heesu0025.kim

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