linux-kernel - Re: [PATCH v3] workqueue: Wrap flush

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: <YohxUC45xk523g9I@slm.duckdns.org>
Date:   Fri, 20 May 2022 18:57:52 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v3] workqueue: Wrap flush_workqueue() using a macro

On Sat, May 21, 2022 at 10:14:36AM +0900, Tetsuo Handa wrote:
> On 2022/05/21 2:10, Tejun Heo wrote:
> > On Fri, May 20, 2022 at 08:43:41PM +0900, Tetsuo Handa wrote:
> >> All flush_workqueue(system_*_wq) users are gone in linux-next.git, and this patch
> >> is for preventing new flush_workqueue(system_*_wq) users from coming in.
> > 
> > Are we fully sure? Also, there can be other changes in flight which aren't
> > covered. It's just not nice in general to intentionally trigger build
> > failures without an easy way to remediate it.
> 
> Yes, we are fully sure. Subset of this patch is already in linux-next.git without problems.
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20220520&id=5015b3b61696f8f44e7113e5bc14f4a20cbf57ff
> There aren't other changes in flight which aren't covered.
> 
> I believe that it is safe to replace the commit above with this patch when Linus released
> 5.18 final (or maybe 5.18-rc8) is released next Sunday. I also believe that it is safe to
> send this patch right before Linus releases 5.19-rc1.
> 
> I guess that there are several out-of-tree kernel modules which will start
> failing with this patch. But they can use
> 
> 	#undef flush_workqueue
> 
> as a temporary workaround (if they can't remediate easily) until we add WARN_ON()
> as a run-time check. We will need to wait for several months until we can add
> WARN_ON() as a run-time check, for that happens after all flush_scheduled_work()
> users are gone.

How is that better tho? If we can just trigger build warning, that's way
better than asking people to hunt down some random define and shoot it down.
How would they do that?

> >> Therefore, triggering a build error (by sending this patch to linux.git right
> >> before 5.19-rc1 in order to make sure that developers will not use
> >> flush_workqueue(system_*_wq) again) is what this patch is for.
> > 
> > What I'm trying to say is that, if we can trigger build warnings, that'd be
> > a better way to go about it.
> 
> Some unlucky users (if any) can workaround this build failure using #undef.
> Nothing to bother with how to emit warning messages instead of error messages.

We're talking in circles. If we can trigger warning, I don't see a reason
why we'd want to trigger build failures. It's a really bad user experience
for anybody who doesn't know what is going on exactly. So, nack on the
current patch.

Thanks.

-- 
tejun