[kernel-hardening] Re: [PATCH v5 04/12] x86: introduce __uaccess_begin_nospec and ifence

All of lore.kernel.org
 help / color / mirror / Atom feed

From: Ingo Molnar <mingo@kernel.org>
To: Dan Williams <dan.j.williams@intel.com>
Cc: tglx@linutronix.de, linux-arch@vger.kernel.org,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Andi Kleen <ak@linux.intel.com>,
	Kees Cook <keescook@chromium.org>,
	kernel-hardening@lists.openwall.com, gregkh@linuxfoundation.org,
	x86@kernel.org, Ingo Molnar <mingo@redhat.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	"H. Peter Anvin" <hpa@zytor.com>,
	torvalds@linux-foundation.org, alan@linux.intel.com,
	linux-kernel@vger.kernel.org
Subject: [kernel-hardening] Re: [PATCH v5 04/12] x86: introduce __uaccess_begin_nospec and ifence
Date: Sun, 28 Jan 2018 10:14:37 +0100	[thread overview]
Message-ID: <20180128091437.4lbll5bev7mgdpug@gmail.com> (raw)
In-Reply-To: <151703973427.26578.15693075353773519333.stgit@dwillia2-desk3.amr.corp.intel.com>

* Dan Williams <dan.j.williams@intel.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.

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.

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.

Thanks,

	Ingo

WARNING: multiple messages have this Message-ID (diff)

From: Ingo Molnar <mingo@kernel.org>
To: Dan Williams <dan.j.williams@intel.com>
Cc: tglx@linutronix.de, linux-arch@vger.kernel.org,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Andi Kleen <ak@linux.intel.com>,
	Kees Cook <keescook@chromium.org>,
	kernel-hardening@lists.openwall.com, gregkh@linuxfoundation.org,
	x86@kernel.org, Ingo Molnar <mingo@redhat.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	"H. Peter Anvin" <hpa@zytor.com>,
	torvalds@linux-foundation.org, alan@linux.intel.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 04/12] x86: introduce __uaccess_begin_nospec and ifence
Date: Sun, 28 Jan 2018 10:14:37 +0100	[thread overview]
Message-ID: <20180128091437.4lbll5bev7mgdpug@gmail.com> (raw)
In-Reply-To: <151703973427.26578.15693075353773519333.stgit@dwillia2-desk3.amr.corp.intel.com>

* Dan Williams <dan.j.williams@intel.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.

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.

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.

Thanks,

	Ingo

next prev parent reply	other threads:[~2018-01-28  9:14 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-27  7:55 [kernel-hardening] [PATCH v5 00/12] spectre variant1 mitigations for tip/x86/pti Dan Williams
2018-01-27  7:55 ` Dan Williams
2018-01-27  7:55 ` Dan Williams
2018-01-27  7:55 ` [kernel-hardening] [PATCH v5 01/12] Documentation: document array_idx Dan Williams
2018-01-27  7:55   ` Dan Williams
2018-01-27  7:55 ` [kernel-hardening] [PATCH v5 02/12] array_idx: sanitize speculative array de-references Dan Williams
2018-01-27  7:55   ` Dan Williams
2018-01-28  8:55   ` [kernel-hardening] " Ingo Molnar
2018-01-28  8:55     ` Ingo Molnar
2018-01-28 11:36     ` [kernel-hardening] " Thomas Gleixner
2018-01-28 11:36       ` Thomas Gleixner
2018-01-28 16:28     ` [kernel-hardening] " Dan Williams
2018-01-28 16:28       ` Dan Williams
2018-01-28 18:33       ` [kernel-hardening] " Ingo Molnar
2018-01-28 18:33         ` Ingo Molnar
2018-01-29 16:45         ` Dan Williams
2018-01-28 18:36       ` [kernel-hardening] " Thomas Gleixner
2018-01-28 18:36         ` Thomas Gleixner
2018-01-30  6:29         ` Dan Williams
2018-01-30 19:38           ` Linus Torvalds
2018-01-30 20:13             ` Dan Williams
2018-01-30 20:27               ` Van De Ven, Arjan
2018-01-31  8:03                 ` Ingo Molnar
2018-01-31 14:13                   ` Van De Ven, Arjan
2018-01-31 14:21                     ` Greg KH
2018-01-27  7:55 ` [kernel-hardening] [PATCH v5 03/12] x86: implement array_idx_mask Dan Williams
2018-01-27  7:55   ` Dan Williams
2018-01-28  9:02   ` [kernel-hardening] " Ingo Molnar
2018-01-28  9:02     ` Ingo Molnar
2018-01-27  7:55 ` [kernel-hardening] [PATCH v5 04/12] x86: introduce __uaccess_begin_nospec and ifence Dan Williams
2018-01-27  7:55   ` Dan Williams
2018-01-28  9:06   ` [kernel-hardening] " Ingo Molnar
2018-01-28  9:06     ` Ingo Molnar
2018-01-28  9:14   ` Ingo Molnar [this message]
2018-01-28  9:14     ` Ingo Molnar
2018-01-29 20:41     ` Dan Williams
2018-01-30  6:56       ` Ingo Molnar
2018-01-27  7:55 ` [kernel-hardening] [PATCH v5 05/12] x86, __get_user: use __uaccess_begin_nospec Dan Williams
2018-01-27  7:55   ` Dan Williams
2018-01-28  9:19   ` [kernel-hardening] " Ingo Molnar
2018-01-28  9:19     ` Ingo Molnar
2018-01-27  7:55 ` [kernel-hardening] [PATCH v5 06/12] x86, get_user: use pointer masking to limit speculation Dan Williams
2018-01-27  7:55   ` Dan Williams
2018-01-28  9:25   ` [kernel-hardening] " Ingo Molnar
2018-01-28  9:25     ` Ingo Molnar
2018-01-27  7:55 ` [kernel-hardening] [PATCH v5 07/12] x86: remove the syscall_64 fast-path Dan Williams
2018-01-27  7:55   ` Dan Williams
2018-01-28  9:29   ` [kernel-hardening] " Ingo Molnar
2018-01-28  9:29     ` Ingo Molnar
2018-01-28 15:22     ` [kernel-hardening] " Andy Lutomirski
2018-01-28 15:22       ` Andy Lutomirski
2018-01-27  7:55 ` [kernel-hardening] [PATCH v5 08/12] x86: sanitize sycall table de-references under speculation Dan Williams
2018-01-27  7:55   ` Dan Williams
2018-01-28  9:36   ` [kernel-hardening] " Ingo Molnar
2018-01-28  9:36     ` Ingo Molnar
2018-01-27  7:56 ` [kernel-hardening] [PATCH v5 09/12] vfs, fdtable: prevent bounds-check bypass via speculative execution Dan Williams
2018-01-27  7:56   ` Dan Williams
2018-01-27  7:56 ` [kernel-hardening] [PATCH v5 10/12] kvm, x86: update spectre-v1 mitigation Dan Williams
2018-01-27  7:56   ` Dan Williams
2018-01-27  7:56 ` [kernel-hardening] [PATCH v5 11/12] nl80211: sanitize array index in parse_txq_params Dan Williams
2018-01-27  7:56   ` Dan Williams
2018-01-27  7:56   ` Dan Williams
2018-01-27  7:56 ` [kernel-hardening] [PATCH v5 12/12] x86/spectre: report get_user mitigation for spectre_v1 Dan Williams
2018-01-27  7:56   ` Dan Williams
2018-01-28  9:50   ` [kernel-hardening] " Ingo Molnar
2018-01-28  9:50     ` Ingo Molnar
2018-01-29 22:05     ` Dan Williams
2018-01-31  8:07       ` Ingo Molnar
2018-02-01 20:23         ` Dan Williams
2018-01-27 18:52 ` [kernel-hardening] Re: [PATCH v5 00/12] spectre variant1 mitigations for tip/x86/pti Linus Torvalds
2018-01-27 18:52   ` Linus Torvalds
2018-01-27 18:52   ` Linus Torvalds
2018-01-27 19:26 ` [kernel-hardening] " Dan Williams
2018-01-27 19:26   ` Dan Williams
2018-01-27 19:26   ` Dan Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180128091437.4lbll5bev7mgdpug@gmail.com \
    --to=mingo@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alan@linux.intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Be sure your reply has a Subject: header at the top and a blank line before the message body.

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.