linux-kernel - Re: [RFC] LKMM: Add volatile

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:   Sat, 5 Jun 2021 21:29:03 -0400
From:   Alan Stern <stern@...land.harvard.edu>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Will Deacon <will@...nel.org>,
        Andrea Parri <parri.andrea@...il.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Nick Piggin <npiggin@...il.com>,
        David Howells <dhowells@...hat.com>,
        Jade Alglave <j.alglave@....ac.uk>,
        Luc Maranget <luc.maranget@...ia.fr>,
        Akira Yokosawa <akiyks@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-toolchains@...r.kernel.org,
        linux-arch <linux-arch@...r.kernel.org>
Subject: Re: [RFC] LKMM: Add volatile_if()

On Sat, Jun 05, 2021 at 05:14:18PM -0700, Paul E. McKenney wrote:
> On Sat, Jun 05, 2021 at 10:57:39AM -0400, Alan Stern wrote:
> > Indeed, the expansion of the currently proposed version of
> > 
> > 	volatile_if (A) {
> > 		B;
> > 	} else {
> > 		C;
> > 	}
> > 
> > is basically the same as
> > 
> > 	if (A) {
> > 		barrier();
> > 		B;
> > 	} else {
> > 		barrier();
> > 		C;
> > 	}

> That does sound good, but...
> 
> Current compilers beg to differ at -O2: https://godbolt.org/z/5K55Gardn
> 
> ------------------------------------------------------------------------
> #define READ_ONCE(x) (*(volatile typeof(x) *)&(x))
> #define WRITE_ONCE(x, val) (READ_ONCE(x) = (val))
> #define barrier() __asm__ __volatile__("": : :"memory")
> 
> int x, y;
> 
> int main(int argc, char *argv[])
> {
>     if (READ_ONCE(x)) {
>         barrier();
>         WRITE_ONCE(y, 1);
>     } else {
>         barrier();
>         WRITE_ONCE(y, 1);
>     }
>     return 0;
> }
> ------------------------------------------------------------------------
> 
> Both gcc and clang generate a load followed by a store, with no branch.
> ARM gets the same results from both compilers.
> 
> As Linus suggested, removing one (but not both!) invocations of barrier()
> does cause a branch to be emitted, so maybe that is a way forward.
> Assuming it is more than just dumb luck, anyway.  :-/

Interesting.  And changing one of the branches from barrier() to __asm__ 
__volatile__("nop": : :"memory") also causes a branch to be emitted.  So 
even though the compiler doesn't "look inside" assembly code, it does 
compare two pieces at least textually and apparently assumes if they are 
identical then they do the same thing.

Alan