From: HeeSu Kim <heesu0025.kim@lge.com>
Port PID namespace conversion logic from C binder to the Rust
implementation.
Without namespace conversion, freeze operations from non-init namespaces
can match wrong processes due to PID collision. This adds proper
conversion to ensure freeze operations target the correct process.
Signed-off-by: HeeSu Kim <heesu0025.kim@lge.com>
---
v2 -> v3:
- Use task::Pid typedef instead of u32/i32
- Use PidNamespace::init_ns() instead of init_pid_ns()
- Compare PidNamespace directly with == instead of raw pointers
- Use Pid::find_vpid() and pid.pid_task() (dropped _with_guard suffix)
- Fix rustfmt import ordering (rcu before Arc)
- Rename TaskPid alias to PidT for clearer pid_t type indication
- Use task.group_leader().pid() instead of tgid_nr_ns() for consistency with C
drivers/android/binder/process.rs | 37 +++++++++++++++++++++++++++----
1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 132055b4790f..ea30bfac2e0b 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -22,6 +22,8 @@
id_pool::IdPool,
list::{List, ListArc, ListArcField, ListLinks},
mm,
+ pid::Pid,
+ pid_namespace::PidNamespace,
prelude::*,
rbtree::{self, RBTree, RBTreeNode, RBTreeNodeReservation},
seq_file::SeqFile,
@@ -29,9 +31,9 @@
sync::poll::PollTable,
sync::{
lock::{spinlock::SpinLockBackend, Guard},
- Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, SpinLock, UniqueArc,
+ rcu, Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, SpinLock, UniqueArc,
},
- task::Task,
+ task::{Pid as PidT, Task},
types::ARef,
uaccess::{UserSlice, UserSliceReader},
uapi,
@@ -1498,17 +1500,42 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
}
}
+/// Convert a PID from the current namespace to the global (init) namespace.
+fn convert_to_init_ns_tgid(pid: PidT) -> Result<PidT> {
+ let current = kernel::current!();
+ let init_ns = PidNamespace::init_ns();
+
+ if current.active_pid_ns() == Some(init_ns) {
+ // Already in init namespace.
+ return Ok(pid);
+ }
+
+ if pid == 0 {
+ return Err(EINVAL);
+ }
+
+ let rcu_guard = rcu::read_lock();
+
+ let pid_struct = Pid::find_vpid(pid, &rcu_guard).ok_or(ESRCH)?;
+ let task = pid_struct.pid_task(&rcu_guard).ok_or(ESRCH)?;
+
+ Ok(task.group_leader().pid())
+}
+
fn get_frozen_status(data: UserSlice) -> Result {
let (mut reader, mut writer) = data.reader_writer();
let mut info = reader.read::<BinderFrozenStatusInfo>()?;
+
+ let init_ns_pid = convert_to_init_ns_tgid(info.pid as PidT)?;
+
info.sync_recv = 0;
info.async_recv = 0;
let mut found = false;
for ctx in crate::context::get_all_contexts()? {
ctx.for_each_proc(|proc| {
- if proc.task.pid() == info.pid as _ {
+ if proc.task.pid() == init_ns_pid as _ {
found = true;
let inner = proc.inner.lock();
let txns_pending = inner.txns_pending_locked();
@@ -1530,13 +1557,15 @@ fn get_frozen_status(data: UserSlice) -> Result {
fn ioctl_freeze(reader: &mut UserSliceReader) -> Result {
let info = reader.read::<BinderFreezeInfo>()?;
+ let init_ns_pid = convert_to_init_ns_tgid(info.pid as PidT)?;
+
// Very unlikely for there to be more than 3, since a process normally uses at most binder and
// hwbinder.
let mut procs = KVec::with_capacity(3, GFP_KERNEL)?;
let ctxs = crate::context::get_all_contexts()?;
for ctx in ctxs {
- for proc in ctx.get_procs_with_pid(info.pid as i32)? {
+ for proc in ctx.get_procs_with_pid(init_ns_pid)? {
procs.push(proc, GFP_KERNEL)?;
}
}
--
2.25.1On Tue Feb 3, 2026 at 6:59 AM GMT, jongan.kim wrote:
> From: HeeSu Kim <heesu0025.kim@lge.com>
>
> Port PID namespace conversion logic from C binder to the Rust
> implementation.
>
> Without namespace conversion, freeze operations from non-init namespaces
> can match wrong processes due to PID collision. This adds proper
> conversion to ensure freeze operations target the correct process.
>
> Signed-off-by: HeeSu Kim <heesu0025.kim@lge.com>
> ---
> v2 -> v3:
> - Use task::Pid typedef instead of u32/i32
> - Use PidNamespace::init_ns() instead of init_pid_ns()
> - Compare PidNamespace directly with == instead of raw pointers
> - Use Pid::find_vpid() and pid.pid_task() (dropped _with_guard suffix)
> - Fix rustfmt import ordering (rcu before Arc)
> - Rename TaskPid alias to PidT for clearer pid_t type indication
> - Use task.group_leader().pid() instead of tgid_nr_ns() for consistency with C
>
> drivers/android/binder/process.rs | 37 +++++++++++++++++++++++++++----
> 1 file changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
> index 132055b4790f..ea30bfac2e0b 100644
> --- a/drivers/android/binder/process.rs
> +++ b/drivers/android/binder/process.rs
> @@ -22,6 +22,8 @@
> id_pool::IdPool,
> list::{List, ListArc, ListArcField, ListLinks},
> mm,
> + pid::Pid,
> + pid_namespace::PidNamespace,
> prelude::*,
> rbtree::{self, RBTree, RBTreeNode, RBTreeNodeReservation},
> seq_file::SeqFile,
> @@ -29,9 +31,9 @@
> sync::poll::PollTable,
> sync::{
> lock::{spinlock::SpinLockBackend, Guard},
> - Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, SpinLock, UniqueArc,
> + rcu, Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, SpinLock, UniqueArc,
> },
> - task::Task,
> + task::{Pid as PidT, Task},
> types::ARef,
> uaccess::{UserSlice, UserSliceReader},
> uapi,
> @@ -1498,17 +1500,42 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
> }
> }
>
> +/// Convert a PID from the current namespace to the global (init) namespace.
> +fn convert_to_init_ns_tgid(pid: PidT) -> Result<PidT> {
> + let current = kernel::current!();
> + let init_ns = PidNamespace::init_ns();
> +
> + if current.active_pid_ns() == Some(init_ns) {
> + // Already in init namespace.
> + return Ok(pid);
> + }
> +
> + if pid == 0 {
> + return Err(EINVAL);
> + }
> +
> + let rcu_guard = rcu::read_lock();
> +
> + let pid_struct = Pid::find_vpid(pid, &rcu_guard).ok_or(ESRCH)?;
> + let task = pid_struct.pid_task(&rcu_guard).ok_or(ESRCH)?;
> +
> + Ok(task.group_leader().pid())
> +}
> +
> fn get_frozen_status(data: UserSlice) -> Result {
> let (mut reader, mut writer) = data.reader_writer();
>
> let mut info = reader.read::<BinderFrozenStatusInfo>()?;
> +
> + let init_ns_pid = convert_to_init_ns_tgid(info.pid as PidT)?;
> +
> info.sync_recv = 0;
> info.async_recv = 0;
> let mut found = false;
>
> for ctx in crate::context::get_all_contexts()? {
> ctx.for_each_proc(|proc| {
> - if proc.task.pid() == info.pid as _ {
> + if proc.task.pid() == init_ns_pid as _ {
> found = true;
> let inner = proc.inner.lock();
> let txns_pending = inner.txns_pending_locked();
> @@ -1530,13 +1557,15 @@ fn get_frozen_status(data: UserSlice) -> Result {
> fn ioctl_freeze(reader: &mut UserSliceReader) -> Result {
> let info = reader.read::<BinderFreezeInfo>()?;
>
> + let init_ns_pid = convert_to_init_ns_tgid(info.pid as PidT)?;
> +
> // Very unlikely for there to be more than 3, since a process normally uses at most binder and
> // hwbinder.
> let mut procs = KVec::with_capacity(3, GFP_KERNEL)?;
>
> let ctxs = crate::context::get_all_contexts()?;
> for ctx in ctxs {
> - for proc in ctx.get_procs_with_pid(info.pid as i32)? {
> + for proc in ctx.get_procs_with_pid(init_ns_pid)? {
I think this function already has `ARef<Task>` for all the process, so it feels
to me that the search should be simply based on task, rather than PID.
I.e. instead of pid_t -> struct pid -> struct tasks -> pid_t and then search
with it, we first get `Task` from VPID, and then search directly with it. This
would also make it no longer necessary to have the init namespace check.
(BTW, the current impl looks quite inefficient, get_procs_with_pid returns a vec
which is pushed again to a vec, perhaps using a callback or simply passing in a
`&mut Vec` is better?)
Best,
Gary
> procs.push(proc, GFP_KERNEL)?;
> }
> }
On Tue Feb 3, 2026 at 12:59:45PM +0000, Gary Guo wrote:
> On Tue Feb 3, 2026 at 6:59 AM GMT, jongan.kim wrote:
> > From: HeeSu Kim <heesu0025.kim@lge.com>
> >
> > Port PID namespace conversion logic from C binder to the Rust
> > implementation.
> >
> > Without namespace conversion, freeze operations from non-init namespaces
> > can match wrong processes due to PID collision. This adds proper
> > conversion to ensure freeze operations target the correct process.
> >
> > Signed-off-by: HeeSu Kim <heesu0025.kim@lge.com>
> > ---
> > v2 -> v3:
> > - Use task::Pid typedef instead of u32/i32
> > - Use PidNamespace::init_ns() instead of init_pid_ns()
> > - Compare PidNamespace directly with == instead of raw pointers
> > - Use Pid::find_vpid() and pid.pid_task() (dropped _with_guard suffix)
> > - Fix rustfmt import ordering (rcu before Arc)
> > - Rename TaskPid alias to PidT for clearer pid_t type indication
> > - Use task.group_leader().pid() instead of tgid_nr_ns() for consistency with C
> >
> > drivers/android/binder/process.rs | 37 +++++++++++++++++++++++++++----
> > 1 file changed, 33 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
> > index 132055b4790f..ea30bfac2e0b 100644
> > --- a/drivers/android/binder/process.rs
> > +++ b/drivers/android/binder/process.rs
> > @@ -22,6 +22,8 @@
> > id_pool::IdPool,
> > list::{List, ListArc, ListArcField, ListLinks},
> > mm,
> > + pid::Pid,
> > + pid_namespace::PidNamespace,
> > prelude::*,
> > rbtree::{self, RBTree, RBTreeNode, RBTreeNodeReservation},
> > seq_file::SeqFile,
> > @@ -29,9 +31,9 @@
> > sync::poll::PollTable,
> > sync::{
> > lock::{spinlock::SpinLockBackend, Guard},
> > - Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, SpinLock, UniqueArc,
> > + rcu, Arc, ArcBorrow, CondVar, CondVarTimeoutResult, Mutex, SpinLock, UniqueArc,
> > },
> > - task::Task,
> > + task::{Pid as PidT, Task},
> > types::ARef,
> > uaccess::{UserSlice, UserSliceReader},
> > uapi,
> > @@ -1498,17 +1500,42 @@ pub(crate) fn ioctl_freeze(&self, info: &BinderFreezeInfo) -> Result {
> > }
> > }
> >
> > +/// Convert a PID from the current namespace to the global (init) namespace.
> > +fn convert_to_init_ns_tgid(pid: PidT) -> Result<PidT> {
> > + let current = kernel::current!();
> > + let init_ns = PidNamespace::init_ns();
> > +
> > + if current.active_pid_ns() == Some(init_ns) {
> > + // Already in init namespace.
> > + return Ok(pid);
> > + }
> > +
> > + if pid == 0 {
> > + return Err(EINVAL);
> > + }
> > +
> > + let rcu_guard = rcu::read_lock();
> > +
> > + let pid_struct = Pid::find_vpid(pid, &rcu_guard).ok_or(ESRCH)?;
> > + let task = pid_struct.pid_task(&rcu_guard).ok_or(ESRCH)?;
> > +
> > + Ok(task.group_leader().pid())
> > +}
> > +
> > fn get_frozen_status(data: UserSlice) -> Result {
> > let (mut reader, mut writer) = data.reader_writer();
> >
> > let mut info = reader.read::<BinderFrozenStatusInfo>()?;
> > +
> > + let init_ns_pid = convert_to_init_ns_tgid(info.pid as PidT)?;
> > +
> > info.sync_recv = 0;
> > info.async_recv = 0;
> > let mut found = false;
> >
> > for ctx in crate::context::get_all_contexts()? {
> > ctx.for_each_proc(|proc| {
> > - if proc.task.pid() == info.pid as _ {
> > + if proc.task.pid() == init_ns_pid as _ {
> > found = true;
> > let inner = proc.inner.lock();
> > let txns_pending = inner.txns_pending_locked();
> > @@ -1530,13 +1557,15 @@ fn get_frozen_status(data: UserSlice) -> Result {
> > fn ioctl_freeze(reader: &mut UserSliceReader) -> Result {
> > let info = reader.read::<BinderFreezeInfo>()?;
> >
> > + let init_ns_pid = convert_to_init_ns_tgid(info.pid as PidT)?;
> > +
> > // Very unlikely for there to be more than 3, since a process normally uses at most binder and
> > // hwbinder.
> > let mut procs = KVec::with_capacity(3, GFP_KERNEL)?;
> >
> > let ctxs = crate::context::get_all_contexts()?;
> > for ctx in ctxs {
> > - for proc in ctx.get_procs_with_pid(info.pid as i32)? {
> > + for proc in ctx.get_procs_with_pid(init_ns_pid)? {
>
> I think this function already has `ARef<Task>` for all the process, so it feels
> to me that the search should be simply based on task, rather than PID.
>
> I.e. instead of pid_t -> struct pid -> struct tasks -> pid_t and then search
> with it, we first get `Task` from VPID, and then search directly with it. This
> would also make it no longer necessary to have the init namespace check.
>
> (BTW, the current impl looks quite inefficient, get_procs_with_pid returns a vec
> which is pushed again to a vec, perhaps using a callback or simply passing in a
> `&mut Vec` is better?)
>
> Best,
> Gary
>
> > procs.push(proc, GFP_KERNEL)?;
> > }
> > }
Thank you for the suggestions for more efficient structure.
Your proposal to use task-based search instead of PID-based search
could simplify the logic.
However, I have a few considerations:
1. The current implementation maintains consistency with the existing
C binder implementation. Any structural change here would ideally be
reflected in the C code as well to keep both implementations aligned.
2. Since the majority of binder usage occurs in the init namespace, the
current early return (checking task_is_in_init_pid_ns) avoids the overhead
of find_vpid() and pid_task() calls in the common case. If we always go
through the task lookup path, this optimization would be lost.
3. Regarding the search mechanism and vec efficiency improvements you
mentioned, these seem like valuable optimizations but would represent
a broader architectural change to the binder driver.
I'd like to get input from the binder driver maintainers on whether:
- This kind of structural change is desired for the binder driver
- If so, whether it should be done as part of this PID namespace fix or
as a separate refactoring effort
- Whether both C and Rust implementations should be updated together
Alice, could you please advise on the preferred approach here?
Thanks,
JongAn Kim
On Wed, Feb 04, 2026 at 06:11:47PM +0900, jongan.kim@lge.com wrote: > On Tue Feb 3, 2026 at 12:59:45PM +0000, Gary Guo wrote: > > On Tue Feb 3, 2026 at 6:59 AM GMT, jongan.kim wrote: > > I think this function already has `ARef<Task>` for all the process, so it feels > > to me that the search should be simply based on task, rather than PID. > > > > I.e. instead of pid_t -> struct pid -> struct tasks -> pid_t and then search > > with it, we first get `Task` from VPID, and then search directly with it. This > > would also make it no longer necessary to have the init namespace check. > > > > (BTW, the current impl looks quite inefficient, get_procs_with_pid returns a vec > > which is pushed again to a vec, perhaps using a callback or simply passing in a > > `&mut Vec` is better?) > > > > Best, > > Gary > > > > > procs.push(proc, GFP_KERNEL)?; > > > } > > > } > > Thank you for the suggestions for more efficient structure. > > Your proposal to use task-based search instead of PID-based search > could simplify the logic. > > However, I have a few considerations: > 1. The current implementation maintains consistency with the existing > C binder implementation. Any structural change here would ideally be > reflected in the C code as well to keep both implementations aligned. Yes, the drivers should match. > 2. Since the majority of binder usage occurs in the init namespace, the > current early return (checking task_is_in_init_pid_ns) avoids the overhead > of find_vpid() and pid_task() calls in the common case. If we always go > through the task lookup path, this optimization would be lost. Well, I suggested this approach too on the previous version: https://lore.kernel.org/lkml/20260130015427.83556-1-jongan.kim@lge.com/ (I mentioned it on C Binder, but they should of course match.) I'm not necessarily that concerned about the optimization. Freezing is already a bit expensive to begin with. > 3. Regarding the search mechanism and vec efficiency improvements you > mentioned, these seem like valuable optimizations but would represent > a broader architectural change to the binder driver. Improving get_procs_with_pid() seems like a separate thing. Perhaps it could be part of this other series: https://lore.kernel.org/all/20260201000817.275382-1-shivamklr@cock.li/ Improving it here should only happen if you need to rewrite said function *anyway*. > I'd like to get input from the binder driver maintainers on whether: > - This kind of structural change is desired for the binder driver > - If so, whether it should be done as part of this PID namespace fix or > as a separate refactoring effort See above. > - Whether both C and Rust implementations should be updated together > > Alice, could you please advise on the preferred approach here? Both implementations should be updated together. Alice
On Wed, Feb 04 2026 10:50:31AM +0000, Alice Ryhl wrote: > On Wed, Feb 04, 2026 at 06:11:47PM +0900, jongan.kim@lge.com wrote: > > On Tue Feb 3, 2026 at 12:59:45PM +0000, Gary Guo wrote: > > > On Tue Feb 3, 2026 at 6:59 AM GMT, jongan.kim wrote: > > > I think this function already has `ARef<Task>` for all the process, so it feels > > > to me that the search should be simply based on task, rather than PID. > > > > > > I.e. instead of pid_t -> struct pid -> struct tasks -> pid_t and then search > > > with it, we first get `Task` from VPID, and then search directly with it. This > > > would also make it no longer necessary to have the init namespace check. > > > > > > (BTW, the current impl looks quite inefficient, get_procs_with_pid returns a vec > > > which is pushed again to a vec, perhaps using a callback or simply passing in a > > > `&mut Vec` is better?) > > > > > > Best, > > > Gary > > > > > > > procs.push(proc, GFP_KERNEL)?; > > > > } > > > > } > > > > Thank you for the suggestions for more efficient structure. > > > > Your proposal to use task-based search instead of PID-based search > > could simplify the logic. > > > > However, I have a few considerations: > > 1. The current implementation maintains consistency with the existing > > C binder implementation. Any structural change here would ideally be > > reflected in the C code as well to keep both implementations aligned. > > Yes, the drivers should match. > > > 2. Since the majority of binder usage occurs in the init namespace, the > > current early return (checking task_is_in_init_pid_ns) avoids the overhead > > of find_vpid() and pid_task() calls in the common case. If we always go > > through the task lookup path, this optimization would be lost. > > Well, I suggested this approach too on the previous version: > https://lore.kernel.org/lkml/20260130015427.83556-1-jongan.kim@lge.com/ > (I mentioned it on C Binder, but they should of course match.) > > I'm not necessarily that concerned about the optimization. Freezing is > already a bit expensive to begin with. Sorry, I misunderstood your intention before. I will update the new patch to apply your recommendations for both the Rust and C. Thank you for the review and suggestions. > > 3. Regarding the search mechanism and vec efficiency improvements you > > mentioned, these seem like valuable optimizations but would represent > > a broader architectural change to the binder driver. > > Improving get_procs_with_pid() seems like a separate thing. Perhaps it > could be part of this other series: > https://lore.kernel.org/all/20260201000817.275382-1-shivamklr@cock.li/ > > Improving it here should only happen if you need to rewrite said > function *anyway*. Since we're changing the implementation to use task-based comparison instead of PID-based comparison, we won't be using get_procs_with_pid() anymore and will need to write a new function (e.g., get_procs_with_task()). Would this approach align with the Rust binder development direction? > > I'd like to get input from the binder driver maintainers on whether: > > - This kind of structural change is desired for the binder driver > > - If so, whether it should be done as part of this PID namespace fix or > > as a separate refactoring effort > > See above. > > > - Whether both C and Rust implementations should be updated together > > > > Alice, could you please advise on the preferred approach here? > > Both implementations should be updated together. We will update both the Rust and C implementations together. Thanks. Jong An, Kim.
On Thu, Feb 05, 2026 at 02:01:28PM +0900, jongan.kim@lge.com wrote: > On Wed, Feb 04 2026 10:50:31AM +0000, Alice Ryhl wrote: > > On Wed, Feb 04, 2026 at 06:11:47PM +0900, jongan.kim@lge.com wrote: > > > 3. Regarding the search mechanism and vec efficiency improvements you > > > mentioned, these seem like valuable optimizations but would represent > > > a broader architectural change to the binder driver. > > > > Improving get_procs_with_pid() seems like a separate thing. Perhaps it > > could be part of this other series: > > https://lore.kernel.org/all/20260201000817.275382-1-shivamklr@cock.li/ > > > > Improving it here should only happen if you need to rewrite said > > function *anyway*. > > Since we're changing the implementation to use task-based comparison instead > of PID-based comparison, we won't be using get_procs_with_pid() anymore and > will need to write a new function (e.g., get_procs_with_task()). > > Would this approach align with the Rust binder development direction? Replacing get_procs_with_pid() with a new get_procs_with_task() sounds reasonable to me. Alice
© 2016 - 2026 Red Hat, Inc.