On 2/24/26 08:02, Andrei Vagin wrote:
> On Mon, Feb 23, 2026 at 3:03 PM Pavel Tikhomirov
> <[email protected]> wrote:
>>
>> This effectively gives us an ability to create the pid namespace init as
>> a child of the process (setns-ed to the pid namespace) different to the
>> process which created the pid namespace itself.
>>
>> Original problem:
>>
>> There is a cool set_tid feature in clone3() syscall, it allows you to
>> create process with desired pids on multiple pid namespace levels. Which
>> is useful to restore processes in CRIU for nested pid namespace case.
>>
>> In nested container case we can potentially see this kind of pid/user
>> namespace tree:
>>
>> Process
>> ┌─────────┐
>> User NS0 ──▶ Pid NS0 ──▶ Pid p0 │
>> │ │ │ │
>> ▼ ▼ │ │
>> User NS1 ──▶ Pid NS1 ──▶ Pid p1 │
>> │ │ │ │
>> ... ... │ ... │
>> │ │ │ │
>> ▼ ▼ │ │
>> User NSn ──▶ Pid NSn ──▶ Pid pn │
>> └─────────┘
>>
>> So to create the "Process" and set pids {p0, p1, ... pn} for it on all
>> pid namespace levels we can use clone3() syscall set_tid feature, BUT
>> the syscall does not allow you to set pid on pid namespace levels you
>> don't have permission to. So basically you have to be in "User NS0" when
>> creating the "Process" to actually be able to set pids on all levels.
>>
>> It is ok for almost any process, but with pid namespace init this does
>> not work, as currently we can only create pid namespace init and the pid
>> namespace itself simultaneously, so to make "Pid NSn" owned by "User
>> NSn" we have to be in the "User NSn".
>>
>> We can't possibly be in "User NS0" and "User NSn" at the same time,
>> hence the problem.
>>
>> Alternative solution:
>>
>> Yes, for the case of pid namespace init we can use old and gold
>> /proc/sys/kernel/ns_last_pid interface on the levels lower than n. But
>> it is much more complicated and introduces tons of extra code to do. It
>> would be nice to make clone3() set_tid interface also aplicable to this
>> corner case.
>>
>> Implementation:
>>
>> Now when anyone can setns to the pid namespace before the creation of
>> init, and thus multiple processes can fork children to the pid
>> namespace, we enforce that the first process created is always the init,
>> and only allow other processes after the init sets
>> pid_namespace->child_reaper.
>>
>> To avoid possible problems related to cpu/compiler optimizations around
>> ->child_reaper, let's use WRITE_ONCE (additional to task_list lock)
>> everywhere we write it and use READ_ONCE everywhere we read it without
>> explicit lock. Note: we already had READ_ONCE in nsfs_fh_to_dentry().
>>
>> Signed-off-by: Pavel Tikhomirov <[email protected]>
>>
>> --
>> v2: Use *_ONCE for ->child_reaper accesses atomicity, and avoid taking
>> task_list lock for reading it. Rebase to master, and thus remove
>> now excess pidns_ready variable.
>>
>> Note: I didn't find anything in copy_process() around setting the
>> ->child_reaper which can influence the pid namespace, so it looks like
>> the pid namespace is fully setup at the point when init sets
>> ->child_reaper to receive more processes. Thus tasklist lock looks
>> excess in pidns_for_children_get()'s ->child_reaper check and it should
>> be safe not to have it in the corresponding checks in alloc_pid().
>> ---
>> kernel/exit.c | 2 +-
>> kernel/fork.c | 2 +-
>> kernel/pid.c | 5 +++--
>> kernel/pid_namespace.c | 9 ---------
>> 4 files changed, 5 insertions(+), 13 deletions(-)
>>
>> diff --git a/kernel/exit.c b/kernel/exit.c
>> index 8a87021211ae..567fc3b7b0f9 100644
>> --- a/kernel/exit.c
>> +++ b/kernel/exit.c
>> @@ -608,7 +608,7 @@ static struct task_struct *find_child_reaper(struct
>> task_struct *father,
>>
>> reaper = find_alive_thread(father);
>> if (reaper) {
>> - pid_ns->child_reaper = reaper;
>> + WRITE_ONCE(pid_ns->child_reaper, reaper);
>> return reaper;
>> }
>>
>> diff --git a/kernel/fork.c b/kernel/fork.c
>> index e832da9d15a4..27d0cdbca67e 100644
>> --- a/kernel/fork.c
>> +++ b/kernel/fork.c
>> @@ -2423,7 +2423,7 @@ __latent_entropy struct task_struct *copy_process(
>> init_task_pid(p, PIDTYPE_SID, task_session(current));
>>
>> if (is_child_reaper(pid)) {
>> - ns_of_pid(pid)->child_reaper = p;
>> + WRITE_ONCE(ns_of_pid(pid)->child_reaper, p);
>> p->signal->flags |= SIGNAL_UNKILLABLE;
>> }
>> p->signal->shared_pending.signal = delayed.signal;
>> diff --git a/kernel/pid.c b/kernel/pid.c
>> index 3b96571d0fe6..e6116e131d8d 100644
>> --- a/kernel/pid.c
>> +++ b/kernel/pid.c
>> @@ -219,7 +219,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t
>> *arg_set_tid,
>> * Also fail if a PID != 1 is requested and
>> * no PID 1 exists.
>> */
>> - if (tid != 1 && !tmp->child_reaper)
>> + if (tid != 1 && !READ_ONCE(tmp->child_reaper))
>> goto out_abort;
>> retval = -EPERM;
>> if (!checkpoint_restore_ns_capable(tmp->user_ns))
>> @@ -247,8 +247,9 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t
>> *arg_set_tid,
>> * alreay in use. Return EEXIST in that case.
>> */
>> if (nr == -ENOSPC)
>> -
>> nr = -EEXIST;
>> + } else if (!READ_ONCE(tmp->child_reaper) &&
>> idr_get_cursor(&tmp->idr) != 0) {
>
> I think it is better to update pid_ns_ctl_handler to prevent setting
> ns_last_pid in a pidns
> without init. Otherwise, figuring out why fork returns EINVAL can be tricky.
Hm, I think pid_ns_ctl_handler(), as it uses current active pid namespace can
only work if current is already fully (ns/pid) in the pid namespace, and thus
the init is also already there. So it's implicitly protected from change
before init creation.
This check here is more for the concurrent alloc_pid() case. When one process
in alloc_pid() successfully allocated the pid and than, for instance, hit the
pidfs_add_pid() error and is going to free_pid(), but the pid 1 is remains yet
allocated from idr and the cursor is on 2 at the moment. At the same time the
concurrent process may get to alloc_pid(), and will see cursor on 2, it should
not be able to create a process as this process will get pid 2 and will be
created before init.
And in general (non concurrent case) it makes sense to only allow allocating 1,
for the first process.
>
>> + nr = -EINVAL;
>> } else {
>> int pid_min = 1;
>> /*
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
