linux-kernel - Re: [PATCH] checkpatch: warn about flushing system-wide workqueues

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: <ed8f50726d0a8b8e5dc9da14ce64039891b77b53.camel@perches.com>
Date:   Thu, 05 May 2022 08:48:31 -0700
From:   Joe Perches <joe@...ches.com>
To:     Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
        Tejun Heo <tj@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andy Whitcroft <apw@...onical.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
Subject: Re: [PATCH] checkpatch: warn about flushing system-wide workqueues

On Thu, 2022-05-05 at 22:42 +0900, Tetsuo Handa wrote:
> On 2022/04/25 9:33, Tetsuo Handa wrote:
> > On 2022/04/25 8:45, Joe Perches wrote:
> > > And it's probably more readable using separate lines and it looks as
> > > if the 3rd test is unnecessary as it could be combined with the 2nd.
> > > 
> > > 		if ($line =~ /\bflush_scheduled_work\s*\(/ ||
> > > 		    $line =~ /\bflush_workqueue\s*\(\s*system(_\w*)?_wq\s*\)/) {
> > 
> > We don't need to worry about possibility like
> > 
> > 	flush_workqueue(system_module1_wq);
> > 
> > ? Then, we can simplify like you suggested.
> 
> I initially thought that also doing static checks by scripts/checkpatch.pl
> is better than only doing runtime WARN_ON(). But not all patches are checked
> by scripts/checkpatch.pl . Thus, as an attempt to check without exemptions,
> I now think that doing static checks via BUILD_BUG_ON() is better than
> scripts/checkpatch.pl . I sent below patch to linux-next.git , and so far
> it seems working (I mean, no build failure reports caused by compilers).
> 
> Subject: workqueue: Wrap flush_workqueue() using a macro
> 
> A conversion to stop flushing kernel-global workqueues is in progress.
> Wrap flush_workqueue() and inject BUILD_BUG_ON() checks, in order to
> prevent users who are not aware of this conversion from again start
> flushing kernel-global workqueues.
[]
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
[]
> @@ -2813,6 +2813,7 @@ static void warn_flush_attempt(struct workqueue_struct *wq)
>   * This function sleeps until all work items which were queued on entry
>   * have finished execution, but it is not livelocked by new incoming ones.
>   */
> +#undef flush_workqueue
>  void flush_workqueue(struct workqueue_struct *wq)

The #undef flush_workqueue could be removed by using

void (flush_workqueue)(struct workqueue_struct *wq)
{
	...
}