Re: [Patch v2 1/4] x86/speculation: Option to select app to app mitigation for spectre_v2

[Tim Chen]

On 10/02/2018 02:23 AM, Ingo Molnar wrote:
> 
> * Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx> 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)
> 
> ?

Will do.

> 
>> +

>>  
>> +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?

This is only used in the core kernel functions related to
context switches.  So I don't expect any module functions
needing this value.

> 
>> +
>>  #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?

Sure, I can add a none option. I'll probably put that in patch 4
which is easy to disable the mitigation by not turning on
STIBP flag for the none option.

> 
> 
>> +	 * 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
> 

Tim