linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] x86/speculation: Clarify Spectre-v2 mitigation when STIBP/IBPB features are unsupported
@ 2020-01-21 16:02 Vitaly Kuznetsov
  2020-01-21 16:14 ` Borislav Petkov
  2020-01-21 16:22 ` Waiman Long
  0 siblings, 2 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2020-01-21 16:02 UTC (permalink / raw)
  To: x86
  Cc: linux-kernel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Josh Poimboeuf, Tyler Hicks, Waiman Long,
	Peter Zijlstra

When STIBP/IBPB features are not supported (no microcode update,
AWS/Azure/... instances deliberately hiding SPEC_CTRL for performance
reasons,...) /sys/devices/system/cpu/vulnerabilities/spectre_v2 looks like

  Mitigation: Full generic retpoline, STIBP: disabled, RSB filling

and this looks imperfect. In particular, STIBP is 'disabled' and 'IBPB'
is not mentioned while both features are just not supported. Also, for
STIBP the 'disabled' state (SPECTRE_V2_USER_NONE) can represent both
the absence of hardware support and deliberate user's choice
(spectre_v2_user=off)

Make the following adjustments:
- Output 'unsupported' for both STIBP/IBPB when there's no support in
  hardware.
- Output 'unneeded' for STIBP when SMT is disabled/missing (and this
  switch_to_cond_stibp is off).

RFC. Some tools out there may be looking at this information so by
changing the output we're breaking them. Also, it may make sense to
separate kernel and userspace protections and switch to something like

  Mitigation: Kernel: Full generic retpoline, RSB filling; Userspace:
   Vulnerable

for the above mentioned case.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 Documentation/admin-guide/hw-vuln/spectre.rst | 3 +++
 arch/x86/kernel/cpu/bugs.c                    | 9 +++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/hw-vuln/spectre.rst b/Documentation/admin-guide/hw-vuln/spectre.rst
index e05e581af5cf..2b8a42d0c57b 100644
--- a/Documentation/admin-guide/hw-vuln/spectre.rst
+++ b/Documentation/admin-guide/hw-vuln/spectre.rst
@@ -385,6 +385,7 @@ The possible values in this file are:
     an x86 only feature. For more details see below.
 
   ===================   ========================================================
+  'IBPB: unsupported'   IBPB is not supported by hardware
   'IBPB: disabled'      IBPB unused
   'IBPB: always-on'     Use IBPB on all tasks
   'IBPB: conditional'   Use IBPB on SECCOMP or indirect branch restricted tasks
@@ -396,6 +397,8 @@ The possible values in this file are:
     only feature. For more details see below.
 
   ====================  ========================================================
+  'STIBP: unsupported'  STIBP is not supported by hardware
+  'STIBP: unneeded'     STIBP is not needed because SMT is disabled
   'STIBP: disabled'     STIBP unused
   'STIBP: forced'       Use STIBP on all tasks
   'STIBP: conditional'  Use STIBP on SECCOMP or indirect branch restricted tasks
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 8bf64899f56a..d72a36fe042b 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1496,7 +1496,10 @@ static char *stibp_state(void)
 
 	switch (spectre_v2_user) {
 	case SPECTRE_V2_USER_NONE:
-		return ", STIBP: disabled";
+		if (boot_cpu_has(X86_FEATURE_STIBP))
+			return ", STIBP: disabled";
+		else
+			return ", STIBP: unsupported";
 	case SPECTRE_V2_USER_STRICT:
 		return ", STIBP: forced";
 	case SPECTRE_V2_USER_STRICT_PREFERRED:
@@ -1505,6 +1508,8 @@ static char *stibp_state(void)
 	case SPECTRE_V2_USER_SECCOMP:
 		if (static_key_enabled(&switch_to_cond_stibp))
 			return ", STIBP: conditional";
+		else
+			return ", STIBP: unneeded";
 	}
 	return "";
 }
@@ -1518,7 +1523,7 @@ static char *ibpb_state(void)
 			return ", IBPB: conditional";
 		return ", IBPB: disabled";
 	}
-	return "";
+	return ", IBPB: unsupported";
 }
 
 static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr,
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC] x86/speculation: Clarify Spectre-v2 mitigation when STIBP/IBPB features are unsupported
  2020-01-21 16:02 [PATCH RFC] x86/speculation: Clarify Spectre-v2 mitigation when STIBP/IBPB features are unsupported Vitaly Kuznetsov
@ 2020-01-21 16:14 ` Borislav Petkov
  2020-01-21 16:24   ` Vitaly Kuznetsov
  2020-01-21 16:22 ` Waiman Long
  1 sibling, 1 reply; 5+ messages in thread
From: Borislav Petkov @ 2020-01-21 16:14 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Josh Poimboeuf, Tyler Hicks, Waiman Long, Peter Zijlstra

On Tue, Jan 21, 2020 at 05:02:57PM +0100, Vitaly Kuznetsov wrote:
> When STIBP/IBPB features are not supported (no microcode update,
> AWS/Azure/... instances deliberately hiding SPEC_CTRL for performance
> reasons,...) /sys/devices/system/cpu/vulnerabilities/spectre_v2 looks like
> 
>   Mitigation: Full generic retpoline, STIBP: disabled, RSB filling
> 
> and this looks imperfect. In particular, STIBP is 'disabled' and 'IBPB'
> is not mentioned while both features are just not supported. Also, for
> STIBP the 'disabled' state (SPECTRE_V2_USER_NONE) can represent both
> the absence of hardware support and deliberate user's choice
> (spectre_v2_user=off)
> 
> Make the following adjustments:
> - Output 'unsupported' for both STIBP/IBPB when there's no support in
>   hardware.
> - Output 'unneeded' for STIBP when SMT is disabled/missing (and this
>   switch_to_cond_stibp is off).
> 
> RFC. Some tools out there may be looking at this information so by
> changing the output we're breaking them. Also, it may make sense to
> separate kernel and userspace protections and switch to something like
> 
>   Mitigation: Kernel: Full generic retpoline, RSB filling; Userspace:
>    Vulnerable
> 
> for the above mentioned case.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  Documentation/admin-guide/hw-vuln/spectre.rst | 3 +++
>  arch/x86/kernel/cpu/bugs.c                    | 9 +++++++--
>  2 files changed, 10 insertions(+), 2 deletions(-)

There's another attempt to fix similar aspects of this whole deal going
on ATM:

https://lkml.kernel.org/r/20191229164830.62144-1-asteinhauser@google.com

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC] x86/speculation: Clarify Spectre-v2 mitigation when STIBP/IBPB features are unsupported
  2020-01-21 16:02 [PATCH RFC] x86/speculation: Clarify Spectre-v2 mitigation when STIBP/IBPB features are unsupported Vitaly Kuznetsov
  2020-01-21 16:14 ` Borislav Petkov
@ 2020-01-21 16:22 ` Waiman Long
  2020-01-21 16:37   ` Vitaly Kuznetsov
  1 sibling, 1 reply; 5+ messages in thread
From: Waiman Long @ 2020-01-21 16:22 UTC (permalink / raw)
  To: Vitaly Kuznetsov, x86
  Cc: linux-kernel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Josh Poimboeuf, Tyler Hicks, Peter Zijlstra

On 1/21/20 11:02 AM, Vitaly Kuznetsov wrote:
> When STIBP/IBPB features are not supported (no microcode update,
> AWS/Azure/... instances deliberately hiding SPEC_CTRL for performance
> reasons,...) /sys/devices/system/cpu/vulnerabilities/spectre_v2 looks like
>
>   Mitigation: Full generic retpoline, STIBP: disabled, RSB filling
>
> and this looks imperfect. In particular, STIBP is 'disabled' and 'IBPB'
> is not mentioned while both features are just not supported. Also, for
> STIBP the 'disabled' state (SPECTRE_V2_USER_NONE) can represent both
> the absence of hardware support and deliberate user's choice
> (spectre_v2_user=off)
>
> Make the following adjustments:
> - Output 'unsupported' for both STIBP/IBPB when there's no support in
>   hardware.
> - Output 'unneeded' for STIBP when SMT is disabled/missing (and this
>   switch_to_cond_stibp is off).

I support outputting "unsupported" when the microcode doesn't support
it. However, I am not sure if "unneeded" is really necessary or not.
STIBP is not needed when SMT is disabled or when Enhanced IBRS is
available and used. Your patch handles the first case, but not the
second. I think it may be easier to just leave it out in case it is not
needed.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC] x86/speculation: Clarify Spectre-v2 mitigation when STIBP/IBPB features are unsupported
  2020-01-21 16:14 ` Borislav Petkov
@ 2020-01-21 16:24   ` Vitaly Kuznetsov
  0 siblings, 0 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2020-01-21 16:24 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Josh Poimboeuf, Tyler Hicks, Waiman Long, Peter Zijlstra,
	Anthony Steinhauser

Borislav Petkov <bp@alien8.de> writes:

> On Tue, Jan 21, 2020 at 05:02:57PM +0100, Vitaly Kuznetsov wrote:
>> When STIBP/IBPB features are not supported (no microcode update,
>> AWS/Azure/... instances deliberately hiding SPEC_CTRL for performance
>> reasons,...) /sys/devices/system/cpu/vulnerabilities/spectre_v2 looks like
>> 
>>   Mitigation: Full generic retpoline, STIBP: disabled, RSB filling
>> 
>> and this looks imperfect. In particular, STIBP is 'disabled' and 'IBPB'
>> is not mentioned while both features are just not supported. Also, for
>> STIBP the 'disabled' state (SPECTRE_V2_USER_NONE) can represent both
>> the absence of hardware support and deliberate user's choice
>> (spectre_v2_user=off)
>> 
>> Make the following adjustments:
>> - Output 'unsupported' for both STIBP/IBPB when there's no support in
>>   hardware.
>> - Output 'unneeded' for STIBP when SMT is disabled/missing (and this
>>   switch_to_cond_stibp is off).
>> 
>> RFC. Some tools out there may be looking at this information so by
>> changing the output we're breaking them. Also, it may make sense to
>> separate kernel and userspace protections and switch to something like
>> 
>>   Mitigation: Kernel: Full generic retpoline, RSB filling; Userspace:
>>    Vulnerable
>> 
>> for the above mentioned case.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>>  Documentation/admin-guide/hw-vuln/spectre.rst | 3 +++
>>  arch/x86/kernel/cpu/bugs.c                    | 9 +++++++--
>>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> There's another attempt to fix similar aspects of this whole deal going
> on ATM:
>
> https://lkml.kernel.org/r/20191229164830.62144-1-asteinhauser@google.com

Missed that, thanks! (Cc: Anthony)

This patch seem to address my STIBP: disabled/unsupported concern but
not 'unneeded'. And not IBPB.

-- 
Vitaly


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC] x86/speculation: Clarify Spectre-v2 mitigation when STIBP/IBPB features are unsupported
  2020-01-21 16:22 ` Waiman Long
@ 2020-01-21 16:37   ` Vitaly Kuznetsov
  0 siblings, 0 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2020-01-21 16:37 UTC (permalink / raw)
  To: Waiman Long, x86
  Cc: linux-kernel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Josh Poimboeuf, Tyler Hicks, Peter Zijlstra

Waiman Long <longman@redhat.com> writes:

> On 1/21/20 11:02 AM, Vitaly Kuznetsov wrote:
>> When STIBP/IBPB features are not supported (no microcode update,
>> AWS/Azure/... instances deliberately hiding SPEC_CTRL for performance
>> reasons,...) /sys/devices/system/cpu/vulnerabilities/spectre_v2 looks like
>>
>>   Mitigation: Full generic retpoline, STIBP: disabled, RSB filling
>>
>> and this looks imperfect. In particular, STIBP is 'disabled' and 'IBPB'
>> is not mentioned while both features are just not supported. Also, for
>> STIBP the 'disabled' state (SPECTRE_V2_USER_NONE) can represent both
>> the absence of hardware support and deliberate user's choice
>> (spectre_v2_user=off)
>>
>> Make the following adjustments:
>> - Output 'unsupported' for both STIBP/IBPB when there's no support in
>>   hardware.
>> - Output 'unneeded' for STIBP when SMT is disabled/missing (and this
>>   switch_to_cond_stibp is off).
>
> I support outputting "unsupported" when the microcode doesn't support
> it. However, I am not sure if "unneeded" is really necessary or not.
> STIBP is not needed when SMT is disabled or when Enhanced IBRS is
> available and used. Your patch handles the first case, but not the
> second. I think it may be easier to just leave it out in case it is not
> needed.

Makes sense. Or, alternatively, we can output 'unneeded' in both cases
to make things explicit and  to distinguish it from the current state of
IBPB where missing means 'unsupported by hardware'.

-- 
Vitaly


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-01-21 16:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-21 16:02 [PATCH RFC] x86/speculation: Clarify Spectre-v2 mitigation when STIBP/IBPB features are unsupported Vitaly Kuznetsov
2020-01-21 16:14 ` Borislav Petkov
2020-01-21 16:24   ` Vitaly Kuznetsov
2020-01-21 16:22 ` Waiman Long
2020-01-21 16:37   ` Vitaly Kuznetsov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).