linux-kernel - Re: re. Spurious wakeup on a newly created kthread

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]

Date:   Sat, 25 Jun 2022 11:25:21 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Tejun Heo <tj@...nel.org>, Petr Mladek <pmladek@...e.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Michal Hocko <mhocko@...e.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Oleg Nesterov <oleg@...hat.com>
Subject: Re: re. Spurious wakeup on a newly created kthread

On Sat, Jun 25, 2022 at 10:36 AM Eric W. Biederman
<ebiederm@...ssion.com> wrote:
>
> Let me suggest someone create a new variant of kthread_create that takes
> all of the parameters the workqueue code wants to set.

I suspect the real issue is that that the kthread code simply
shouldn't use the kernel_thread() helper at all.

That helper is literally designed for "start a thread, run this thing".

That's what it *does*.

And that's not at all what the kthread code wants. It wants to set
affinity masks, it wants to create a name for the thread, it wants to
do all those other things.

That code really wants to just do copy_process().

Or maybe it really should just use create_io_thread(), which has a
much better interface, except it has that one oddity in that it sets
the flag that does this:

        if (args->io_thread) {
                /*
                 * Mark us an IO worker, and block any signal that isn't
                 * fatal or STOP
                 */
                p->flags |= PF_IO_WORKER;
                siginitsetinv(&p->blocked, sigmask(SIGKILL)|sigmask(SIGSTOP));
        }

that then has special semantics.

Now, those special semantics may actually be exactly what kthreads
wants, but they are important:

                /*
                 * PF_IO_WORKER threads will catch and exit on fatal signals
                 * themselves. They have cleanup that must be performed, so
                 * we cannot call do_exit() on their behalf.
                 */

which is about that "what happens for fatal signals" thing.

               Linus