linux-kernel - Re: [PATCH v3 1/3] binder: handle PID namespace conversion for freeze operation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aYJc42j9bwgZyCFD@yury>
Date: Tue, 3 Feb 2026 15:38:59 -0500
From: Yury Norov <ynorov@...dia.com>
To: jongan.kim@....com
Cc: a.hindborg@...nel.org, aliceryhl@...gle.com, arve@...roid.com,
	bjorn3_gh@...tonmail.com, boqun.feng@...il.com, brauner@...nel.org,
	cmllamas@...gle.com, dakr@...nel.org, daniel.almeida@...labora.com,
	gary@...yguo.net, gregkh@...uxfoundation.org, tamird@...il.com,
	tkjos@...roid.com, tmgross@...ch.edu, viresh.kumar@...aro.org,
	vitaly.wool@...sulko.se, yury.norov@...il.com, lossin@...nel.org,
	ojeda@...nel.org, heesu0025.kim@....com, ht.hong@....com,
	jungsu.hwang@....com, kernel-team@...roid.com,
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
	sanghun.lee@....com, seulgi.lee@....com, sunghoon.kim@....com
Subject: Re: [PATCH v3 1/3] binder: handle PID namespace conversion for
 freeze operation

On Tue, Feb 03, 2026 at 03:59:26PM +0900, jongan.kim@....com wrote:
> From: JongAn Kim <jongan.kim@....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@....com>
> ---
> v2 -> v3 : change to use task->tgid instead of task_tgid_nr_ns()
> 
>  drivers/android/binder.c | 53 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 535fc881c8da..4c4366089ecb 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5609,6 +5609,41 @@ 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)

For global PIDs we've got task_pid_nr(), see include/linux/pid.h:

 /*
  * the helpers to get the task's different pids as they are seen
  * from various namespaces
  *
  * task_xid_nr()     : global id, i.e. the id seen from the init namespace;
  * task_xid_vnr()    : virtual id, i.e. the id seen from the pid namespace of
  *                     current.
  * task_xid_nr_ns()  : id seen from the ns specified;
  *
  * see also pid_nr() etc in include/linux/pid.h
  */

I think task_tgid_nr(current) would work for you. Or I misunderstand
something?

If your "binder_convert" returns something not covered by one from
the above, please put your function in include/linux/pid.h and give
it a proper name.

> + * @pid:    pid from user space
> + *
> + * Converts a process ID (TGID) from the caller's PID namespace to the
> + * corresponding TGID in the init namespace.

Process ID (PID) is not the same as TGID, but you use the names
interchangeably. This is very confusing. Can you reword?

> + * 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)

This should use pid_t.

> +{
> +	struct task_struct *task;
> +	int init_ns_pid = 0;
> +
> +	/* already in init namespace */
> +	if (task_is_in_init_pid_ns(current))
> +		return pid;
> +
> +	if (pid == 0)
> +		return -EINVAL;

Can you comment what is wrong with pid == 0?

> +	rcu_read_lock();
> +	task = pid_task(find_vpid(pid), PIDTYPE_PID);
> +	if (task)
> +		init_ns_pid = task->tgid;

So I've been replying with the same suggestion to v2, but you did it
in this v3 yourself.

> +	rcu_read_unlock();
> +
> +	if (!init_ns_pid)
> +		return -ESRCH;

You can assign init_ns_pid to -ESRCH at declaration and drop this chunk.

> +
> +	return init_ns_pid;
> +}

Thanks,
Yury