Re: [RFC PATCH] x86/speculation: Don't inherit TIF_SSBD on execve()

[Waiman Long]

On 12/19/2018 02:09 PM, Waiman Long wrote:
> With the default SPEC_STORE_BYPASS_SECCOMP/SPEC_STORE_BYPASS_PRCTL mode,
> the TIF_SSBD bit will be inherited when a new task is fork'ed or cloned.
>
> As only certain class of applications (like Java) requires disabling
> speculative store bypass for security purpose, it may not make sense to
> allow the TIF_SSBD bit to be inherited across execve() boundary where the
> new application may not need SSBD at all and is probably not aware that
> SSBD may have been turned on. This may cause an unnecessary performance
> loss of up to 20% in some cases.
>
> The arch_setup_new_exec() function is updated to clear the TIF_SSBD
> bit unless it has been force-disabled.
>
> Signed-off-by: Waiman Long <longman@xxxxxxxxxx>
> ---
>  Documentation/userspace-api/spec_ctrl.rst |  3 +++
>  arch/x86/kernel/process.c                 | 10 ++++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/Documentation/userspace-api/spec_ctrl.rst b/Documentation/userspace-api/spec_ctrl.rst
> index c4dbe6f..226aed5 100644
> --- a/Documentation/userspace-api/spec_ctrl.rst
> +++ b/Documentation/userspace-api/spec_ctrl.rst
> @@ -55,6 +55,9 @@ is selected by arg2 of :manpage:`prctl(2)` per task. arg3 is used to hand
>  in the control value, i.e. either PR_SPEC_ENABLE or PR_SPEC_DISABLE or
>  PR_SPEC_FORCE_DISABLE.
>  
> +When mitigation is enabled, its state will not be inherited on
> +:manpage:`execve(2)` unless it is force-disabled.
> +
>  Common error codes
>  ------------------
>  ======= =================================================================
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 7d31192..f207f4d 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -252,6 +252,16 @@ void arch_setup_new_exec(void)
>  	/* If cpuid was previously disabled for this task, re-enable it. */
>  	if (test_thread_flag(TIF_NOCPUID))
>  		enable_cpuid();
> +
> +	/*
> +	 * Don't inherit TIF_SSBD across exec boundary unless speculative
> +	 * store bypass is force-disabled (e.g. seccomp on).
> +	 */
> +	if (test_thread_flag(TIF_SSBD) &&
> +	   !task_spec_ssb_force_disable(current)) {
> +		clear_thread_flag(TIF_SSBD);
> +		task_clear_spec_ssb_disable(current);
> +	}
>  }
>  
>  static inline void switch_to_bitmap(struct thread_struct *prev,

Ping! Any comments of objections?

Cheers,
Longman