[v2] x86/bugs: Adjust SRSO mitigation to new features

[PATCH v2 1/4] x86/bugs: Add SRSO_USER_KERNEL_NO support

Posted by Borislav Petkov 1 year, 2 months ago

From: "Borislav Petkov (AMD)" <bp@alien8.de>

If the machine has:

  CPUID Fn8000_0021_EAX[30] (SRSO_USER_KERNEL_NO) -- If this bit is 1,
  it indicates the CPU is not subject to the SRSO vulnerability across
  user/kernel boundaries.

have it fall back to IBPB on VMEXIT only, in the case it is going to run
VMs:

  Speculative Return Stack Overflow: CPU user/kernel transitions protected, falling back to IBPB-on-VMEXIT
  Speculative Return Stack Overflow: Mitigation: IBPB on VMEXIT only

Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kernel/cpu/bugs.c         | 6 ++++++
 arch/x86/kernel/cpu/common.c       | 1 +
 3 files changed, 8 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 17b6590748c0..2787227a8b42 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -464,6 +464,7 @@
 #define X86_FEATURE_SBPB		(20*32+27) /* Selective Branch Prediction Barrier */
 #define X86_FEATURE_IBPB_BRTYPE		(20*32+28) /* MSR_PRED_CMD[IBPB] flushes all branch type predictions */
 #define X86_FEATURE_SRSO_NO		(20*32+29) /* CPU is not affected by SRSO */
+#define X86_FEATURE_SRSO_USER_KERNEL_NO	(20*32+30) /* CPU is not affected by SRSO across user/kernel boundaries */
 
 /*
  * Extended auxiliary flags: Linux defined - for features scattered in various
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 47a01d4028f6..8854d9bce2a5 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2615,6 +2615,11 @@ static void __init srso_select_mitigation(void)
 		break;
 
 	case SRSO_CMD_SAFE_RET:
+		if (boot_cpu_has(X86_FEATURE_SRSO_USER_KERNEL_NO)) {
+			pr_notice("CPU user/kernel transitions protected, falling back to IBPB-on-VMEXIT\n");
+			goto ibpb_on_vmexit;
+		}
+
 		if (IS_ENABLED(CONFIG_MITIGATION_SRSO)) {
 			/*
 			 * Enable the return thunk for generated code
@@ -2658,6 +2663,7 @@ static void __init srso_select_mitigation(void)
 		}
 		break;
 
+ibpb_on_vmexit:
 	case SRSO_CMD_IBPB_ON_VMEXIT:
 		if (IS_ENABLED(CONFIG_MITIGATION_SRSO)) {
 			if (!boot_cpu_has(X86_FEATURE_ENTRY_IBPB) && has_microcode) {
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index a5c28975c608..954f9c727f11 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1270,6 +1270,7 @@ static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
 	VULNBL_AMD(0x17, RETBLEED | SMT_RSB | SRSO),
 	VULNBL_HYGON(0x18, RETBLEED | SMT_RSB | SRSO),
 	VULNBL_AMD(0x19, SRSO),
+	VULNBL_AMD(0x1a, SRSO),
 	{}
 };
 
-- 
2.43.0

Re: [PATCH v2 1/4] x86/bugs: Add SRSO_USER_KERNEL_NO support

Posted by Josh Poimboeuf 1 year, 1 month ago

On Mon, Dec 02, 2024 at 01:04:13PM +0100, Borislav Petkov wrote:
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -2615,6 +2615,11 @@ static void __init srso_select_mitigation(void)
>  		break;
>  
>  	case SRSO_CMD_SAFE_RET:
> +		if (boot_cpu_has(X86_FEATURE_SRSO_USER_KERNEL_NO)) {
> +			pr_notice("CPU user/kernel transitions protected, falling back to IBPB-on-VMEXIT\n");
> +			goto ibpb_on_vmexit;

The presence of SRSO_USER_KERNEL_NO should indeed change the default,
but if the user requests "safe_ret" specifically, shouldn't we give it
to them?  That would be more consistent with how we handle requested
mitigations.

Also it doesn't really make sense to add a printk here as the mitigation
will be printed at the end of the function.

-- 
Josh

Re: [PATCH v2 1/4] x86/bugs: Add SRSO_USER_KERNEL_NO support

Posted by Borislav Petkov 1 year, 1 month ago

On Mon, Dec 09, 2024 at 10:53:31PM -0800, Josh Poimboeuf wrote:
> The presence of SRSO_USER_KERNEL_NO should indeed change the default,
> but if the user requests "safe_ret" specifically, shouldn't we give it
> to them?

Hardly a valid use case except for debugging but if you're doing that, you can
change the kernel too.

Because we just fixed this and if some poor soul has left
spec_rstack_overflow=safe-ret in her /etc/default/grub (it has happened to me
a bunch on my test boxes), she'll never get her performance back and that
would be a yucky situation.

> That would be more consistent with how we handle requested
> mitigations.

Yeah, but if there were a point to this, I guess. We don't really need this
mitigation as there's nothing to mitigate on the user/kernel transition
anymore.

> Also it doesn't really make sense to add a printk here as the mitigation
> will be printed at the end of the function.

This is us letting the user know that we don't need Safe-RET anymore and we're
falling back. But I'm not that hung up on that printk...

Thx.

-- 
Regards/Gruss,
    Boris.

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

[tip: x86/bugs] x86/bugs: Add SRSO_USER_KERNEL_NO support

Posted by tip-bot2 for Borislav Petkov (AMD) 1 year, 1 month ago

The following commit has been merged into the x86/bugs branch of tip:

Commit-ID:     877818802c3e970f67ccb53012facc78bef5f97a
Gitweb:        https://git.kernel.org/tip/877818802c3e970f67ccb53012facc78bef5f97a
Author:        Borislav Petkov (AMD) <bp@alien8.de>
AuthorDate:    Mon, 11 Nov 2024 17:22:08 +01:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Mon, 30 Dec 2024 17:48:33 +01:00

x86/bugs: Add SRSO_USER_KERNEL_NO support

If the machine has:

  CPUID Fn8000_0021_EAX[30] (SRSO_USER_KERNEL_NO) -- If this bit is 1,
  it indicates the CPU is not subject to the SRSO vulnerability across
  user/kernel boundaries.

have it fall back to IBPB on VMEXIT only, in the case it is going to run
VMs:

  Speculative Return Stack Overflow: Mitigation: IBPB on VMEXIT only

Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
Link: https://lore.kernel.org/r/20241202120416.6054-2-bp@kernel.org
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kernel/cpu/bugs.c         | 4 ++++
 arch/x86/kernel/cpu/common.c       | 1 +
 3 files changed, 6 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 645aa36..0e2d817 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -465,6 +465,7 @@
 #define X86_FEATURE_SBPB		(20*32+27) /* Selective Branch Prediction Barrier */
 #define X86_FEATURE_IBPB_BRTYPE		(20*32+28) /* MSR_PRED_CMD[IBPB] flushes all branch type predictions */
 #define X86_FEATURE_SRSO_NO		(20*32+29) /* CPU is not affected by SRSO */
+#define X86_FEATURE_SRSO_USER_KERNEL_NO	(20*32+30) /* CPU is not affected by SRSO across user/kernel boundaries */
 
 /*
  * Extended auxiliary flags: Linux defined - for features scattered in various
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 47a01d4..5a505aa 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2615,6 +2615,9 @@ static void __init srso_select_mitigation(void)
 		break;
 
 	case SRSO_CMD_SAFE_RET:
+		if (boot_cpu_has(X86_FEATURE_SRSO_USER_KERNEL_NO))
+			goto ibpb_on_vmexit;
+
 		if (IS_ENABLED(CONFIG_MITIGATION_SRSO)) {
 			/*
 			 * Enable the return thunk for generated code
@@ -2658,6 +2661,7 @@ static void __init srso_select_mitigation(void)
 		}
 		break;
 
+ibpb_on_vmexit:
 	case SRSO_CMD_IBPB_ON_VMEXIT:
 		if (IS_ENABLED(CONFIG_MITIGATION_SRSO)) {
 			if (!boot_cpu_has(X86_FEATURE_ENTRY_IBPB) && has_microcode) {
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 3e90376..7e8d811 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1270,6 +1270,7 @@ static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
 	VULNBL_AMD(0x17, RETBLEED | SMT_RSB | SRSO),
 	VULNBL_HYGON(0x18, RETBLEED | SMT_RSB | SRSO),
 	VULNBL_AMD(0x19, SRSO),
+	VULNBL_AMD(0x1a, SRSO),
 	{}
 };