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: <YrjAJN7dDJ9R7Ocu@mtj.duckdns.org>
Date:   Mon, 27 Jun 2022 05:23:00 +0900
From:   Tejun Heo <tj@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     "Eric W. Biederman" <ebiederm@...ssion.com>,
        Christian Brauner <brauner@...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: [PATCH 3/3] kthread: Stop abusing TASK_UNINTERRUPTIBLE
 (INCOMPLETE)

Hello,

On Sun, Jun 26, 2022 at 12:59:09PM -0700, Linus Torvalds wrote:
> And I think *that* should be the change - add a "setup()" function
> pointer to the whole kthread infrastructure. Allow it to return an
> error, which will then just kill the new thread again without ever
> even starting it up.
> 
> I'd really prefer to avoid having random drivers and subsystems know
> about the *very* magical "wake_up_new_task()" thing.  Yes, it's a real
> thing, but it's a thing that normal code should not ever use.
> 
> The whole "wake_up_process()" model for kthread creation was wrong.
> But moving existing users of a bad interface to using the even more
> special "wake_up_new_task()" thing is not the solution, I feel.

This is a bit of bike-shedding but there are inherent downsides to
callback based interface in terms of write/readability. Now you have
to move the init code out of line, and if the context that needs to be
passing doesn't fit in a single pointer, you gotta define a struct to
carry it adding to the boilerplate.

Having separate create and run steps makes sense as a pattern and
wake_up_new_task() is less error prone in one sense - if the caller
doesn't call it, the new kthread actually won't start running making
the bug obvious. The fact that the function blindly trusts the caller
to be handing in an actual new task is scary and we likely don't wanna
proliferate its use across the code base.

Would it be a reasonable tradeoff to have a kthread wrapper -
kthread_start() or whatever - which ensures that it is actually called
once on a new task? That way, we can keep the init code inline and
bugs on both sides (not starting and starting incorrectly) are
obvious.

Thanks.

-- 
tejun

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ