On Mon, Feb 20, 2023 at 10:27 AM Josh Poimboeuf jpoimboe@kernel.org wrote:
IBRS is only enabled in kernel space. Since it's not enabled in user space, user space isn't protected from indirect branch prediction attacks from a sibling CPU thread.
Allow STIBP to be enabled to protect against such attacks.
Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS") Reported-by: José Oliveira joseloliveira11@gmail.com Reported-by: Rodrigo Branco rodrigo@kernelhacking.com Signed-off-by: Josh Poimboeuf jpoimboe@kernel.org
arch/x86/kernel/cpu/bugs.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index 85168740f76a..b97c0d28e573 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -1124,14 +1124,19 @@ spectre_v2_parse_user_cmdline(void) return SPECTRE_V2_USER_CMD_AUTO; }
-static inline bool spectre_v2_in_ibrs_mode(enum spectre_v2_mitigation mode) +static inline bool spectre_v2_in_eibrs_mode(enum spectre_v2_mitigation mode) {
return mode == SPECTRE_V2_IBRS ||mode == SPECTRE_V2_EIBRS ||
return mode == SPECTRE_V2_EIBRS || mode == SPECTRE_V2_EIBRS_RETPOLINE || mode == SPECTRE_V2_EIBRS_LFENCE;}
There are no comments here, this code is in dire need for some comments and explanation, I was trying something like:
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index bca0bd8f4846..3e04f9fa68a8 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -1124,14 +1124,31 @@ spectre_v2_parse_user_cmdline(void) return SPECTRE_V2_USER_CMD_AUTO; }
-static inline bool spectre_v2_in_ibrs_mode(enum spectre_v2_mitigation mode) +static inline bool spectre_v2_in_eibrs_mode(enum spectre_v2_mitigation mode) { - return mode == SPECTRE_V2_IBRS || - mode == SPECTRE_V2_EIBRS || + return mode == SPECTRE_V2_EIBRS || mode == SPECTRE_V2_EIBRS_RETPOLINE || mode == SPECTRE_V2_EIBRS_LFENCE; }
+/* + * In IBRS mode, the spectre_v2 mitigation is enabled only in kernel space with + * the IBRS bit being cleared on return to userspace due to performance + * overhead. + */ +static inline bool spectre_v2_in_ibrs_mode(enum spectre_v2_mitigation mode) +{ + return spectre_v2_in_eibrs_mode(mode) || mode == SPECTRE_V2_IBRS; +} + +/* + * User mode protections are only available in eIBRS mode. + */ +static inline bool spectre_v2_user_needs_stibp(enum spectre_v2_mitigation mode) +{ + return !spectre_v2_in_eibrs_mode(mode); +} + static void __init spectre_v2_user_select_mitigation(void) { @@ -1193,13 +1210,8 @@ spectre_v2_user_select_mitigation(void) "always-on" : "conditional"); }
- /* - * If no STIBP, IBRS or enhanced IBRS is enabled, or SMT impossible, - * STIBP is not required. - */ - if (!boot_cpu_has(X86_FEATURE_STIBP) || - !smt_possible || - spectre_v2_in_ibrs_mode(spectre_v2_enabled)) + if (!boot_cpu_has(X86_FEATURE_STIBP) || !smt_possible || + !spectre_v2_user_needs_stibp(spectre_v2_enabled)) return;
/* @@ -2327,7 +2339,7 @@ static ssize_t mmio_stale_data_show_state(char *buf)
static char *stibp_state(void) { - if (spectre_v2_in_ibrs_mode(spectre_v2_enabled)) + if (!spectre_v2_user_needs_stibp(spectre_v2_enabled)) return "";
switch (spectre_v2_user_stibp) {
Also Josh, is it okay for us to have a discussion and have me write the patch as a v2? Your current patch does not even credit me at all. Seems a bit unfair, but I don't really care. I was going to rev up the patch with your suggestions.
+static inline bool spectre_v2_in_ibrs_mode(enum spectre_v2_mitigation mode) +{
return spectre_v2_in_eibrs_mode(mode) ||mode == SPECTRE_V2_IBRS;+}
static void __init spectre_v2_user_select_mitigation(void) { @@ -1194,12 +1199,12 @@ spectre_v2_user_select_mitigation(void) }
/*
* If no STIBP, IBRS or enhanced IBRS is enabled, or SMT impossible,
* If no STIBP, enhanced IBRS is enabled, or SMT impossible, * STIBP is not required. */ if (!boot_cpu_has(X86_FEATURE_STIBP) || !smt_possible ||
spectre_v2_in_ibrs_mode(spectre_v2_enabled))
spectre_v2_in_eibrs_mode(spectre_v2_enabled)) return; /*@@ -2327,9 +2332,6 @@ static ssize_t mmio_stale_data_show_state(char *buf)
static char *stibp_state(void) {
if (spectre_v2_in_ibrs_mode(spectre_v2_enabled))return "";switch (spectre_v2_user_stibp) { case SPECTRE_V2_USER_NONE: return ", STIBP: disabled";-- 2.39.1