linux-kernel - Re: [PATCH] x86: Add an explicit barrier() to clflushopt()

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:	Thu, 7 Jan 2016 19:44:13 +0000
From:	Chris Wilson <chris@...is-wilson.co.uk>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Ross Zwisler <ross.zwisler@...ux.intel.com>,
	"H . Peter Anvin" <hpa@...ux.intel.com>,
	Borislav Petkov <bp@...en8.de>,
	Brian Gerst <brgerst@...il.com>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Imre Deak <imre.deak@...el.com>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	DRI <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH] x86: Add an explicit barrier() to clflushopt()

On Thu, Jan 07, 2016 at 09:55:51AM -0800, Andy Lutomirski wrote:
> On Thu, Jan 7, 2016 at 2:16 AM, Chris Wilson <chris@...is-wilson.co.uk> wrote:
> >> +     /* GCC (4.9.1 and 5.2.1 at least) appears to be very confused when
> >> +      * meeting this alternative() and demonstrably miscompiles loops
> >> +      * iterating over clflushopts.
> >> +      */
> >> +     barrier();
> >>  }
> >
> > Or an alternative:
> >
> > +#define alternative_output(oldinstr, newinstr, feature, output)        \
> > +       asm volatile (ALTERNATIVE(oldinstr, newinstr, feature)          \
> > +               : output : "i" (0) : "memory")
> >
> > I would really appreciate some knowledgeable folks taking a look at the
> > asm for clflushopt() as it still affects today's kernel and gcc.
> >
> > Fwiw, I have confirmed that arch/x86/mm/pageattr.c clflush_cache_range()
> > is similarly affected.
> 
> Unless I'm mis-reading the asm, clflush_cache_range() is compiled
> correctly for me.  (I don't know what the %P is for in the asm, but
> that shouldn't matter.)  The ALTERNATIVE shouldn't even be visible to
> the optimizer.
> 
> Can you attach a bad .s file and let us know what gcc version this is?
>  (You can usually do 'make foo/bar/baz.s' to get a .s file.)  I'd also
> be curious whether changing clflushopt to clwb works around the issue.

Now I feel silly. Looking at the .s, there is no difference with the
addition of the barrier to clflush_cache_range(). And sure enough
letting the test run for longer, we see a failure. I fell for a placebo.

The failing assertion is always on the last cacheline and is always one
value behind. Oh well, back to wondering where we miss the flush.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre