linux-kernel - Re: Debugging a TTY race condition on M1 (memory ordering dragons)

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: <8ccfdd2d-ef77-4586-e50c-985e1d13726a@marcan.st>
Date:   Tue, 16 Aug 2022 03:26:21 +0900
From:   Hector Martin <marcan@...can.st>
To:     Boqun Feng <boqun.feng@...il.com>
Cc:     Will Deacon <will@...nel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Greg KH <gregkh@...uxfoundation.org>, jirislaby@...nel.org,
        Marc Zyngier <maz@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Asahi Linux <asahi@...ts.linux.dev>,
        Oliver Neukum <oneukum@...e.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: Debugging a TTY race condition on M1 (memory ordering dragons)

On 16/08/2022 03.04, Boqun Feng wrote:
> On Tue, Aug 16, 2022 at 01:01:17AM +0900, Hector Martin wrote:
> Hmm.. but doesn't your (and Will's) finding actually show why
> queue_work() only guarantee ordering if queuing succeeds? In other
> words, if you want extra ordering, use smp_mb() before queue_work()
> like:
> 
> 	smp_mb();	// pairs with smp_mb() in set_work_pool_and_clear_pending()
> 	queue_work();	// if queue_work() return false, it means the work
> 			// is pending, and someone will eventually clear
> 		      	// the pending bit, with the smp_mb() above it's
> 			// guaranteed that work function will see the
> 			// memory accesses above.
> 
> Of course, I shall defer this to workqueue folks. Just saying that it
> may not be broken. We have a few similar guarantees, for example,
> wake_up_process() only provides ordering if it really wakes up a
> process.

Technically yes, but that doesn't actually make a lot of sense, and in
fact the comments inside the workqueue code imply that it does actually
provide order even in the failure case (and there are other barriers to
try to make that happen, just not enough). Note that the ordering
documentation was added post-facto, and I don't think the person who
wrote it necessarily considered whether it *actually* provides
guarantees in the failure case, and whether it should.

wake_up_process() is different because it doesn't actually guarantee
anything if the process is already awake. However, under this
definition, queue_work() guarantees that *some* work execution will
observe every preceding write before queue_work(), regardless of the
current state, and that is a very useful property. That is something
that wake_up_process() semantics can't do.

Without this guarantee, basically every queue_work() user that's using
some kind of producer/consumer pattern would need the explicit barrier.
I imagine that pattern is very common.

- Hector