linux-kernel - Re: [RFC PATCH] x86/speculation: Don't inherit TIF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 7 Jan 2019 09:49:03 -0500
From:   Waiman Long <longman@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...en8.de>,
        Jonathan Corbet <corbet@....net>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, "H. Peter Anvin" <hpa@...or.com>,
        Andi Kleen <ak@...ux.intel.com>,
        David Woodhouse <dwmw@...zon.co.uk>,
        Jiri Kosina <jikos@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        KarimAllah Ahmed <karahmed@...zon.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Subject: Re: [RFC PATCH] x86/speculation: Don't inherit TIF_SSBD on execve()

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@...hat.com>
> ---
>  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