[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 29 Jan 2018 12:41:00 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Ingo Molnar <mingo@...nel.org>
Cc: Thomas Gleixner <tglx@...utronix.de>, linux-arch <linux-arch@...r.kernel.org>,
Tom Lendacky <thomas.lendacky@....com>, Andi Kleen <ak@...ux.intel.com>,
Kees Cook <keescook@...omium.org>,
Kernel Hardening <kernel-hardening@...ts.openwall.com>, Greg KH <gregkh@...uxfoundation.org>,
X86 ML <x86@...nel.org>, Ingo Molnar <mingo@...hat.com>, Al Viro <viro@...iv.linux.org.uk>,
"H. Peter Anvin" <hpa@...or.com>, Linus Torvalds <torvalds@...ux-foundation.org>,
Alan Cox <alan@...ux.intel.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 04/12] x86: introduce __uaccess_begin_nospec and ifence
On Sun, Jan 28, 2018 at 1:14 AM, Ingo Molnar <mingo@...nel.org> wrote:
>
> * Dan Williams <dan.j.williams@...el.com> wrote:
>
>> --- a/arch/x86/include/asm/uaccess.h
>> +++ b/arch/x86/include/asm/uaccess.h
>> @@ -124,6 +124,11 @@ extern int __get_user_bad(void);
>>
>> #define __uaccess_begin() stac()
>> #define __uaccess_end() clac()
>> +#define __uaccess_begin_nospec() \
>> +({ \
>> + stac(); \
>> + ifence(); \
>> +})
>
> BTW., wouldn't it be better to switch the barrier order here, i.e. to do:
>
> ifence(); \
> stac(); \
>
> ?
>
> The reason is that stac()/clac() is usually paired, so there's a chance with short
> sequences that it would resolve with 'no externally visible changes to flags'.
>
> Also, there's many cases where flags are modified _inside_ the STAC/CLAC section,
> so grouping them together inside a speculation atom could be beneficial.
I'm struggling a bit to grok this. Are you referring to the state of
the flags from the address limit comparison? That's the result that
needs fencing before speculative execution leaks to to the user
pointer de-reference.
> The flip side is that if the MFENCE stalls the STAC that is ahead of it could be
> processed for 'free' - while it's always post barrier with my suggestion.
This 'for free' aspect is what I aiming for.
>
> But in any case it would be nice to see a discussion of this aspect in the
> changelog, even if the patch does not change.
I'll add a note to the changelog that having the fence after the
'stac' hopefully allows some overlap of the cost of 'stac' and the
flushing of the instruction pipeline.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
