linux-kernel - Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates

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:   Fri, 16 Aug 2019 14:04:56 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Joel Fernandes <joel@...lfernandes.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        rostedt <rostedt@...dmis.org>,
        Valentin Schneider <valentin.schneider@....com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        paulmck <paulmck@...ux.ibm.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Will Deacon <will.deacon@....com>,
        David Howells <dhowells@...hat.com>
Subject: Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates

On Fri, Aug 16, 2019 at 1:49 PM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> Can we finally put a foot down and tell compiler and standard committee
> people to stop this insanity?

It's already effectively done.

Yes, values can be read from memory multiple times if they need
reloading. So "READ_ONCE()" when the value can change is a damn good
idea.

But it should only be used if the value *can* change. Inside a locked
region it is actively pointless and misleading.

Similarly, WRITE_ONCE() should only be used if you have a _reason_ for
using it (notably if you're not holding a lock).

If people use READ_ONCE/WRITE_ONCE when there are locks that prevent
the values from changing, they are only making the code illegible.
Don't do it.

But in the *absence* of locking, READ_ONCE/WRITE_ONCE is usually a
good thing.  The READ_ONCE actually tends to matter, because even if
the value is used only once at a source level, the compiler *could*
decide to do something else.

The WRITE_ONCE() may or may not matter (afaik, thanks to concurrency,
modern C standard does not allow optimistic writes anyway, and we
wouldn't really accept such a compiler option if it did).

But if the write is done without locking, it's good practice just to
show you are aware of the whole "done without locking" part.

              Linus