linux-kernel - Re: [PATCH] pid: Do not set pid

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: <CAJqdLrri0q-Et5PfdwP69r7wYDi3_nY225UP5HYVABUFtE5TGw@mail.gmail.com>
Date: Thu, 6 Mar 2025 10:11:27 +0100
From: Alexander Mikhalitsyn <alexander@...alicyn.com>
To: Michal Koutný <mkoutny@...e.com>
Cc: Christian Brauner <brauner@...nel.org>, linux-kernel@...r.kernel.org, 
	Joel Granados <joel.granados@...nel.org>, Oleg Nesterov <oleg@...hat.com>, 
	"Eric W . Biederman" <ebiederm@...ssion.com>, "Martin K . Petersen" <martin.petersen@...cle.com>, 
	Wei Liu <wei.liu@...nel.org>, Baoquan He <bhe@...hat.com>, 
	Frederic Weisbecker <frederic@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] pid: Do not set pid_max in new pid namespaces

Am Mi., 5. März 2025 um 15:59 Uhr schrieb Michal Koutný <mkoutny@...e.com>:
>
> It is already difficult for users to troubleshoot which of multiple pid
> limits restricts their workload. The per-(hierarchical-)NS pid_max would
> contribute to the confusion.
> Also, the implementation copies the limit upon creation from
> parent, this pattern showed cumbersome with some attributes in legacy
> cgroup controllers -- it's subject to race condition between parent's
> limit modification and children creation and once copied it must be
> changed in the descendant.
>
> Let's do what other places do (ucounts or cgroup limits) -- create new
> pid namespaces without any limit at all. The global limit (actually any
> ancestor's limit) is still effectively in place, we avoid the
> set/unshare race and bumps of global (ancestral) limit have the desired
> effect on pid namespace that do not care.
>
> Link: https://lore.kernel.org/r/20240408145819.8787-1-mkoutny@suse.com/
> Link: https://lore.kernel.org/r/20250221170249.890014-1-mkoutny@suse.com/
> Fixes: 7863dcc72d0f4 ("pid: allow pid_max to be set per pid namespace")
> Signed-off-by: Michal Koutný <mkoutny@...e.com>

Dear Michal,

This completely makes sense and I tend to agree. But we also need to
ensure that the kselftest for pid_max is not broken with this change.
Let me play with this stuff a bit and I get back with "Tested-by" ;-)

Kind regards,
Alex

> ---
>  kernel/pid_namespace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> index 8f6cfec87555a..7098ed44e717d 100644
> --- a/kernel/pid_namespace.c
> +++ b/kernel/pid_namespace.c
> @@ -107,7 +107,7 @@ static struct pid_namespace *create_pid_namespace(struct user_namespace *user_ns
>                 goto out_free_idr;
>         ns->ns.ops = &pidns_operations;
>
> -       ns->pid_max = parent_pid_ns->pid_max;
> +       ns->pid_max = PID_MAX_LIMIT;
>         err = register_pidns_sysctls(ns);
>         if (err)
>                 goto out_free_inum;
>
> base-commit: 48a5eed9ad584315c30ed35204510536235ce402
> --
> 2.48.1
>