linux-kernel - Re: [PATCH 3/3] rcu-tasks: Fix synchronize_rcu_tasks() VS zap_pid_ns

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: <87ilitpeq1.fsf@email.froward.int.ebiederm.org>
Date:   Fri, 02 Dec 2022 17:28:54 -0600
From:   "Eric W. Biederman" <ebiederm@...ssion.com>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     "Paul E . McKenney" <paulmck@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Neeraj Upadhyay <quic_neeraju@...cinc.com>,
        Oleg Nesterov <oleg@...hat.com>,
        Pengfei Xu <pengfei.xu@...el.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Lai Jiangshan <jiangshanlai@...il.com>, rcu@...r.kernel.org
Subject: Re: [PATCH 3/3] rcu-tasks: Fix synchronize_rcu_tasks() VS
 zap_pid_ns_processes()

Frederic Weisbecker <frederic@...nel.org> writes:

> On Wed, Nov 30, 2022 at 12:37:15PM -0600, Eric W. Biederman wrote:
>> Frederic Weisbecker <frederic@...nel.org> writes:
>> Two questions.
>> 
>> 1) Is there any chance you need the exit_task_rcu_stop() and
>>    exit_tasks_rcu_start() around schedule in the part of this code that
>>    calls kernel_wait4.
>
> Indeed it could be relaxed there too if necessary.

I was just wondering how you tell if it is necessary and if perhaps
the kernel_wait4 was overlooked.

>> 2) I keep thinking zap_pid_ns_processes() should be changed so that
>>    after it sends SIGKILL to all of the relevant processes to not wait,
>>    and instead have wait_consider_task simply not allow the 
>>    init process of the pid namespace to be reaped.
>> 
>>    Am I right in thinking that such a change were to be made it would
>>    make remove the deadlock without having to have any special code?
>> 
>>    It is just tricky enough to do that I don't want to discourage your
>>    simpler change but this looks like a case that makes the pain of
>>    changing zap_pid_ns_processes worthwhile in the practice.
>
> So you mean it still reaps those that were EXIT_ZOMBIE before ignoring
> SIGCHLD (the kernel_wait4() call) but it doesn't sleep anymore on those
> that autoreap (or get reaped by a parent outside that namespace) after
> ignoring SIGCHLD? Namely it doesn't do the schedule() loop I'm working
> around here and proceeds with exit_notify() and notifies its parent?

Yes.  A change to make it work like when the thread group leader exits
before the other threads.  So any actual sleeping happens in the reaper
of the init process when the reaper calls wait(2).

> And then in this case the responsibility of sleeping, until the init_process
> of the namespace is the last task in the namespace, goes to the parent while
> waiting that init_process, right?
>
> But what if the init_process of the given namespace autoreaps? Should it then
> wait itself until the namespace is empty? And then aren't we back to the initial
> issue?

The idea is that we only care when the userspace cares.  I don't think
there is any kernel code that fundamentally cares.  There might be a few
bits that accidentally care and those would need to be take care of when
making the proposed change.  The wait would only exist for userspace so
the same semantics are observed.

Eric