Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 2 Oct 2018 11:23:28 +0200
From:   Ingo Molnar <mingo@kernel.org>
To:     Tim Chen <tim.c.chen@linux.intel.com>
Cc:     Jiri Kosina <jikos@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        David Woodhouse <dwmw@amazon.co.uk>,
        Andi Kleen <ak@linux.intel.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Casey Schaufler <casey.schaufler@intel.com>,
        Asit Mallick <asit.k.mallick@intel.com>,
        Arjan van de Ven <arjan@linux.intel.com>,
        Jon Masters <jcm@redhat.com>, linux-kernel@vger.kernel.org,
        x86@kernel.org
Subject: Re: [Patch v2 1/4] x86/speculation: Option to select app to app
 mitigation for spectre_v2
Message-ID: <20181002092328.GA122128@gmail.com>
References: <cover.1537920575.git.tim.c.chen@linux.intel.com>
 <cb7314d6deb8d1c58e2ead56fef8848b642e4e5a.1537920575.git.tim.c.chen@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <cb7314d6deb8d1c58e2ead56fef8848b642e4e5a.1537920575.git.tim.c.chen@linux.intel.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


* Tim Chen <tim.c.chen@linux.intel.com> wrote:

> Subject: x86/speculation: Option to select app to app mitigation for spectre_v2
>

We prefer to start commit titles with verbs, not nouns, so this should be something like:

  x86/speculation: Add option to select app to app mitigation for spectre_v2

> Jiri Kosina's patch makes IBPB and STIBP available for
> general spectre v2 app to app mitigation.  IBPB will be issued for
> switching to an app that's not ptraceable by the previous
> app and STIBP will be always turned on.
> 
> However, app to app exploit is in general difficult
> due to address space layout randomization in apps and
> the need to know an app's address space layout ahead of time.
> Users may not wish to incur app to app performance
> overhead from IBPB and STIBP for general non security sensitive apps.
> 
> This patch provides a lite option for spectre_v2 app to app
> mitigation where IBPB is only issued for security sensitive
> non-dumpable app.
> 
> The strict option will keep system at high security level
> where IBPB and STIBP are used to defend all apps against
> spectre_v2 app to app attack.

s/system
 /the system

s/attack
  attacks

> +	spectre_v2_app2app=
> +			[X86] Control app to app mitigation of Spectre variant 2
> +			(indirect branch speculation) vulnerability.
> +
> +			lite   - only turn on mitigation for non-dumpable processes
> +			strict - protect against attacks for all user processes
> +			auto   - let kernel decide lite or strict mode

Perhaps add:
			lite   - only turn on mitigation for non-dumpable processes (i.e. 
				 protect daemons and other privileged processes that tend
				 to be non-dumpable)

?

> +
> +			Not specifying this option is equivalent to
> +			spectre_v2_app2app=auto.
> +
>  	spec_store_bypass_disable=
>  			[HW] Control Speculative Store Bypass (SSB) Disable mitigation
>  			(Speculative Store Bypass vulnerability)
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index fd2a8c1..c59a6c4 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -3,6 +3,7 @@
>  #ifndef _ASM_X86_NOSPEC_BRANCH_H_
>  #define _ASM_X86_NOSPEC_BRANCH_H_
>  
> +#include <linux/static_key.h>
>  #include <asm/alternative.h>
>  #include <asm/alternative-asm.h>
>  #include <asm/cpufeatures.h>
> @@ -217,6 +218,12 @@ enum spectre_v2_mitigation {
>  	SPECTRE_V2_IBRS_ENHANCED,
>  };
>  
> +enum spectre_v2_app2app_mitigation {
> +	SPECTRE_V2_APP2APP_NONE,
> +	SPECTRE_V2_APP2APP_LITE,
> +	SPECTRE_V2_APP2APP_STRICT,
> +};
> +
>  /* The Speculative Store Bypass disable variants */
>  enum ssb_mitigation {
>  	SPEC_STORE_BYPASS_NONE,
> @@ -228,6 +235,8 @@ enum ssb_mitigation {
>  extern char __indirect_thunk_start[];
>  extern char __indirect_thunk_end[];
>  
> +DECLARE_STATIC_KEY_FALSE(spectre_v2_app_lite);
> +
>  /*
>   * On VMEXIT we must ensure that no RSB predictions learned in the guest
>   * can be followed in the host, by overwriting the RSB completely. Both
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index ee46dcb..c967012 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -133,6 +133,12 @@ enum spectre_v2_mitigation_cmd {
>  	SPECTRE_V2_CMD_RETPOLINE_AMD,
>  };
>  
> +enum spectre_v2_app2app_mitigation_cmd {
> +	SPECTRE_V2_APP2APP_CMD_AUTO,
> +	SPECTRE_V2_APP2APP_CMD_LITE,
> +	SPECTRE_V2_APP2APP_CMD_STRICT,
> +};
> +
>  static const char *spectre_v2_strings[] = {
>  	[SPECTRE_V2_NONE]			= "Vulnerable",
>  	[SPECTRE_V2_RETPOLINE_MINIMAL]		= "Vulnerable: Minimal generic ASM retpoline",
> @@ -142,12 +148,24 @@ static const char *spectre_v2_strings[] = {
>  	[SPECTRE_V2_IBRS_ENHANCED]		= "Mitigation: Enhanced IBRS",
>  };
>  
> +static const char *spectre_v2_app2app_strings[] = {
> +	[SPECTRE_V2_APP2APP_NONE]		= "App-App Vulnerable",
> +	[SPECTRE_V2_APP2APP_LITE]		= "App-App Mitigation: Protect only non-dumpable process",
> +	[SPECTRE_V2_APP2APP_STRICT]		= "App-App Mitigation: Full app to app attack protection",
> +};
> +
> +DEFINE_STATIC_KEY_FALSE(spectre_v2_app_lite);
> +EXPORT_SYMBOL(spectre_v2_app_lite);

EXPORT_SYMBOL_GPL() I suspect?

> +
>  #undef pr_fmt
>  #define pr_fmt(fmt)     "Spectre V2 : " fmt
>  
>  static enum spectre_v2_mitigation spectre_v2_enabled __ro_after_init =
>  	SPECTRE_V2_NONE;
>  
> +static enum spectre_v2_mitigation spectre_v2_app2app_enabled __ro_after_init =
> +	SPECTRE_V2_APP2APP_NONE;
> +
>  void
>  x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool setguest)
>  {
> @@ -275,6 +293,46 @@ static const struct {
>  	{ "auto",              SPECTRE_V2_CMD_AUTO,              false },
>  };
>  
> +static const struct {
> +	const char *option;
> +	enum spectre_v2_app2app_mitigation_cmd cmd;
> +	bool secure;
> +} app2app_mitigation_options[] = {
> +	{ "lite",	SPECTRE_V2_APP2APP_CMD_LITE,   false },
> +	{ "strict",	SPECTRE_V2_APP2APP_CMD_STRICT, false },
> +	{ "auto",	SPECTRE_V2_APP2APP_CMD_AUTO,   false },
> +};

Am I reading this right that it's not possible to configure this to 'none', i.e. to disable the 
protection altogether?


> +	 * For lite protection mode, we only protect the non-dumpable
> +	 * processes.
> +	 *
> +	 * Otherwise check if the current (previous) task has access to the memory
> +	 * of the @tsk (next) task for strict app to app protection.
> +	 * If access is denied, make sure to
>  	 * issue a IBPB to stop user->user Spectre-v2 attacks.

s/a IBPB
 /an IBPB

Thanks,

	Ingo