| 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
| ||
|
Message-ID: <23d2bdb5-ebe6-b943-5c07-6e3087710651@amd.com>
Date: Thu, 5 Nov 2020 14:32:10 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: Anand K Mistry <amistry@...gle.com>, x86@...nel.org
Cc: asteinhauser@...gle.com, tglx@...utronix.de, joelaf@...gle.com,
Borislav Petkov <bp@...en8.de>,
"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Mark Gross <mgross@...ux.intel.com>,
Mike Rapoport <rppt@...nel.org>,
Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
Tony Luck <tony.luck@...el.com>,
Waiman Long <longman@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] x86/speculation: Allow IBPB to be conditionally
enabled on CPUs with always-on STIBP
On 11/4/20 11:33 PM, Anand K Mistry wrote:
> On AMD CPUs which have the feature X86_FEATURE_AMD_STIBP_ALWAYS_ON,
> STIBP is set to on and 'spectre_v2_user_stibp ==
> SPECTRE_V2_USER_STRICT_PREFERRED'. At the same time, IBPB can be set to
> conditional. However, this leads to the case where it's impossible to
> turn on IBPB for a process because in the PR_SPEC_DISABLE case in
> ib_prctl_set, the (spectre_v2_user_stibp ==
> SPECTRE_V2_USER_STRICT_PREFERRED) condition leads to a return before the
> task flag is set. Similarly, ib_prctl_get will return PR_SPEC_DISABLE
> even though IBPB is set to conditional.
>
> More generally, the following cases are possible:
> 1. STIBP = conditional && IBPB = on for spectre_v2_user=seccomp,ibpb
> 2. STIBP = on && IBPB = conditional for AMD CPUs with
> X86_FEATURE_AMD_STIBP_ALWAYS_ON
>
> The first case functions correctly today, but only because
> spectre_v2_user_ibpb isn't updated to reflect the IBPB mode.
>
> At a high level, this change does one thing. If either STIBP or IBPB is
> set to conditional, allow the prctl to change the task flag. Also,
> reflect that capability when querying the state. This isn't perfect
> since it doesn't take into account if only STIBP or IBPB is
> unconditionally on. But it allows the conditional feature to work as
> expected, without affecting the unconditional one.
>
> Signed-off-by: Anand K Mistry <amistry@...gle.com>
Does it need a Fixes: tag?
Acked-by: Tom Lendacky <thomas.lendacky@....com>
>
> ---
>
> Changes in v2:
> - Fix typo in commit message
> - s/is_spec_ib_user/is_spec_ib_user_controlled
> - Update comment in ib_prctl_set() to reference X86_FEATURE_AMD_STIBP_ALWAYS_ON
> - Have is_spec_ib_user_controlled() check both IBPB and STIBP modes
>
> arch/x86/kernel/cpu/bugs.c | 46 +++++++++++++++++++++++---------------
> 1 file changed, 28 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index d3f0db463f96..534225afe832 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -1254,6 +1254,14 @@ static int ssb_prctl_set(struct task_struct *task, unsigned long ctrl)
> return 0;
> }
>
> +static bool is_spec_ib_user_controlled(void)
> +{
> + return spectre_v2_user_ibpb == SPECTRE_V2_USER_PRCTL ||
> + spectre_v2_user_ibpb == SPECTRE_V2_USER_SECCOMP ||
> + spectre_v2_user_stibp == SPECTRE_V2_USER_PRCTL ||
> + spectre_v2_user_stibp == SPECTRE_V2_USER_SECCOMP;
> +}
> +
> static int ib_prctl_set(struct task_struct *task, unsigned long ctrl)
> {
> switch (ctrl) {
> @@ -1262,13 +1270,20 @@ static int ib_prctl_set(struct task_struct *task, unsigned long ctrl)
> spectre_v2_user_stibp == SPECTRE_V2_USER_NONE)
> return 0;
> /*
> - * Indirect branch speculation is always disabled in strict
> - * mode. It can neither be enabled if it was force-disabled
> - * by a previous prctl call.
> + * With strict mode for both IBPB and STIBP, the instruction
> + * code paths avoid checking this task flag and instead,
> + * unconditionally run the instruction. However, STIBP and IBPB
> + * are independent and either can be set to conditionally
> + * enabled regardless of the mode of the other. If either is set
> + * to conditional, allow the task flag to be updated, unless it
> + * was force-disabled by a previous prctl call.
> + * Currently, this is possible on an AMD CPU which has the
> + * feature X86_FEATURE_AMD_STIBP_ALWAYS_ON. In this case, if the
> + * kernel is booted with 'spectre_v2_user=seccomp', then
> + * spectre_v2_user_ibpb == SPECTRE_V2_USER_SECCOMP and
> + * spectre_v2_user_stibp == SPECTRE_V2_USER_STRICT_PREFERRED.
> */
> - if (spectre_v2_user_ibpb == SPECTRE_V2_USER_STRICT ||
> - spectre_v2_user_stibp == SPECTRE_V2_USER_STRICT ||
> - spectre_v2_user_stibp == SPECTRE_V2_USER_STRICT_PREFERRED ||
> + if (!is_spec_ib_user_controlled() ||
> task_spec_ib_force_disable(task))
> return -EPERM;
> task_clear_spec_ib_disable(task);
> @@ -1283,9 +1298,7 @@ static int ib_prctl_set(struct task_struct *task, unsigned long ctrl)
> if (spectre_v2_user_ibpb == SPECTRE_V2_USER_NONE &&
> spectre_v2_user_stibp == SPECTRE_V2_USER_NONE)
> return -EPERM;
> - if (spectre_v2_user_ibpb == SPECTRE_V2_USER_STRICT ||
> - spectre_v2_user_stibp == SPECTRE_V2_USER_STRICT ||
> - spectre_v2_user_stibp == SPECTRE_V2_USER_STRICT_PREFERRED)
> + if (!is_spec_ib_user_controlled())
> return 0;
> task_set_spec_ib_disable(task);
> if (ctrl == PR_SPEC_FORCE_DISABLE)
> @@ -1351,20 +1364,17 @@ static int ib_prctl_get(struct task_struct *task)
> if (spectre_v2_user_ibpb == SPECTRE_V2_USER_NONE &&
> spectre_v2_user_stibp == SPECTRE_V2_USER_NONE)
> return PR_SPEC_ENABLE;
> - else if (spectre_v2_user_ibpb == SPECTRE_V2_USER_STRICT ||
> - spectre_v2_user_stibp == SPECTRE_V2_USER_STRICT ||
> - spectre_v2_user_stibp == SPECTRE_V2_USER_STRICT_PREFERRED)
> - return PR_SPEC_DISABLE;
> - else if (spectre_v2_user_ibpb == SPECTRE_V2_USER_PRCTL ||
> - spectre_v2_user_ibpb == SPECTRE_V2_USER_SECCOMP ||
> - spectre_v2_user_stibp == SPECTRE_V2_USER_PRCTL ||
> - spectre_v2_user_stibp == SPECTRE_V2_USER_SECCOMP) {
> + else if (is_spec_ib_user_controlled()) {
> if (task_spec_ib_force_disable(task))
> return PR_SPEC_PRCTL | PR_SPEC_FORCE_DISABLE;
> if (task_spec_ib_disable(task))
> return PR_SPEC_PRCTL | PR_SPEC_DISABLE;
> return PR_SPEC_PRCTL | PR_SPEC_ENABLE;
> - } else
> + } else if (spectre_v2_user_ibpb == SPECTRE_V2_USER_STRICT ||
> + spectre_v2_user_stibp == SPECTRE_V2_USER_STRICT ||
> + spectre_v2_user_stibp == SPECTRE_V2_USER_STRICT_PREFERRED)
> + return PR_SPEC_DISABLE;
> + else
> return PR_SPEC_NOT_AFFECTED;
> }
>
>
Powered by blists - more mailing lists