Restructure retbleed mitigation to use select/update/apply functions to
create consistent vulnerability handling. The retbleed_update_mitigation()
simplifies the dependency between spectre_v2 and retbleed.
The command line options now directly select a preferred mitigation
which simplifies the logic.
Signed-off-by: David Kaplan <david.kaplan@amd.com>
---
arch/x86/kernel/cpu/bugs.c | 170 +++++++++++++++++--------------------
1 file changed, 77 insertions(+), 93 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 3d468bd9573f..66abc398d5b4 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -57,6 +57,8 @@ static void __init spectre_v1_select_mitigation(void);
static void __init spectre_v1_apply_mitigation(void);
static void __init spectre_v2_select_mitigation(void);
static void __init retbleed_select_mitigation(void);
+static void __init retbleed_update_mitigation(void);
+static void __init retbleed_apply_mitigation(void);
static void __init spectre_v2_user_select_mitigation(void);
static void __init ssb_select_mitigation(void);
static void __init l1tf_select_mitigation(void);
@@ -180,11 +182,6 @@ void __init cpu_select_mitigations(void)
/* Select the proper CPU mitigations before patching alternatives: */
spectre_v1_select_mitigation();
spectre_v2_select_mitigation();
- /*
- * retbleed_select_mitigation() relies on the state set by
- * spectre_v2_select_mitigation(); specifically it wants to know about
- * spectre_v2=ibrs.
- */
retbleed_select_mitigation();
/*
* spectre_v2_user_select_mitigation() relies on the state set by
@@ -212,12 +209,14 @@ void __init cpu_select_mitigations(void)
* After mitigations are selected, some may need to update their
* choices.
*/
+ retbleed_update_mitigation();
mds_update_mitigation();
taa_update_mitigation();
mmio_update_mitigation();
rfds_update_mitigation();
spectre_v1_apply_mitigation();
+ retbleed_apply_mitigation();
mds_apply_mitigation();
taa_apply_mitigation();
mmio_apply_mitigation();
@@ -1064,6 +1063,7 @@ enum spectre_v2_mitigation spectre_v2_enabled __ro_after_init = SPECTRE_V2_NONE;
enum retbleed_mitigation {
RETBLEED_MITIGATION_NONE,
+ RETBLEED_MITIGATION_AUTO,
RETBLEED_MITIGATION_UNRET,
RETBLEED_MITIGATION_IBPB,
RETBLEED_MITIGATION_IBRS,
@@ -1071,14 +1071,6 @@ enum retbleed_mitigation {
RETBLEED_MITIGATION_STUFF,
};
-enum retbleed_mitigation_cmd {
- RETBLEED_CMD_OFF,
- RETBLEED_CMD_AUTO,
- RETBLEED_CMD_UNRET,
- RETBLEED_CMD_IBPB,
- RETBLEED_CMD_STUFF,
-};
-
static const char * const retbleed_strings[] = {
[RETBLEED_MITIGATION_NONE] = "Vulnerable",
[RETBLEED_MITIGATION_UNRET] = "Mitigation: untrained return thunk",
@@ -1089,9 +1081,7 @@ static const char * const retbleed_strings[] = {
};
static enum retbleed_mitigation retbleed_mitigation __ro_after_init =
- RETBLEED_MITIGATION_NONE;
-static enum retbleed_mitigation_cmd retbleed_cmd __ro_after_init =
- IS_ENABLED(CONFIG_MITIGATION_RETBLEED) ? RETBLEED_CMD_AUTO : RETBLEED_CMD_OFF;
+ IS_ENABLED(CONFIG_MITIGATION_RETBLEED) ? RETBLEED_MITIGATION_AUTO : RETBLEED_MITIGATION_NONE;
static int __ro_after_init retbleed_nosmt = false;
@@ -1108,15 +1098,15 @@ static int __init retbleed_parse_cmdline(char *str)
}
if (!strcmp(str, "off")) {
- retbleed_cmd = RETBLEED_CMD_OFF;
+ retbleed_mitigation = RETBLEED_MITIGATION_NONE;
} else if (!strcmp(str, "auto")) {
- retbleed_cmd = RETBLEED_CMD_AUTO;
+ retbleed_mitigation = RETBLEED_MITIGATION_AUTO;
} else if (!strcmp(str, "unret")) {
- retbleed_cmd = RETBLEED_CMD_UNRET;
+ retbleed_mitigation = RETBLEED_MITIGATION_UNRET;
} else if (!strcmp(str, "ibpb")) {
- retbleed_cmd = RETBLEED_CMD_IBPB;
+ retbleed_mitigation = RETBLEED_MITIGATION_IBPB;
} else if (!strcmp(str, "stuff")) {
- retbleed_cmd = RETBLEED_CMD_STUFF;
+ retbleed_mitigation = RETBLEED_MITIGATION_STUFF;
} else if (!strcmp(str, "nosmt")) {
retbleed_nosmt = true;
} else if (!strcmp(str, "force")) {
@@ -1137,53 +1127,38 @@ early_param("retbleed", retbleed_parse_cmdline);
static void __init retbleed_select_mitigation(void)
{
- bool mitigate_smt = false;
-
- if (!boot_cpu_has_bug(X86_BUG_RETBLEED) || cpu_mitigations_off())
- return;
-
- switch (retbleed_cmd) {
- case RETBLEED_CMD_OFF:
+ if (!boot_cpu_has_bug(X86_BUG_RETBLEED) || cpu_mitigations_off()) {
+ retbleed_mitigation = RETBLEED_MITIGATION_NONE;
return;
+ }
- case RETBLEED_CMD_UNRET:
- if (IS_ENABLED(CONFIG_MITIGATION_UNRET_ENTRY)) {
- retbleed_mitigation = RETBLEED_MITIGATION_UNRET;
- } else {
+ switch (retbleed_mitigation) {
+ case RETBLEED_MITIGATION_UNRET:
+ if (!IS_ENABLED(CONFIG_MITIGATION_UNRET_ENTRY)) {
+ retbleed_mitigation = RETBLEED_MITIGATION_AUTO;
pr_err("WARNING: kernel not compiled with MITIGATION_UNRET_ENTRY.\n");
- goto do_cmd_auto;
}
break;
-
- case RETBLEED_CMD_IBPB:
+ case RETBLEED_MITIGATION_IBPB:
if (!boot_cpu_has(X86_FEATURE_IBPB)) {
pr_err("WARNING: CPU does not support IBPB.\n");
- goto do_cmd_auto;
- } else if (IS_ENABLED(CONFIG_MITIGATION_IBPB_ENTRY)) {
- retbleed_mitigation = RETBLEED_MITIGATION_IBPB;
- } else {
+ retbleed_mitigation = RETBLEED_MITIGATION_AUTO;
+ } else if (!IS_ENABLED(CONFIG_MITIGATION_IBPB_ENTRY)) {
pr_err("WARNING: kernel not compiled with MITIGATION_IBPB_ENTRY.\n");
- goto do_cmd_auto;
+ retbleed_mitigation = RETBLEED_MITIGATION_AUTO;
}
break;
-
- case RETBLEED_CMD_STUFF:
- if (IS_ENABLED(CONFIG_MITIGATION_CALL_DEPTH_TRACKING) &&
- spectre_v2_enabled == SPECTRE_V2_RETPOLINE) {
- retbleed_mitigation = RETBLEED_MITIGATION_STUFF;
-
- } else {
- if (IS_ENABLED(CONFIG_MITIGATION_CALL_DEPTH_TRACKING))
- pr_err("WARNING: retbleed=stuff depends on spectre_v2=retpoline\n");
- else
- pr_err("WARNING: kernel not compiled with MITIGATION_CALL_DEPTH_TRACKING.\n");
-
- goto do_cmd_auto;
+ case RETBLEED_MITIGATION_STUFF:
+ if (!IS_ENABLED(CONFIG_MITIGATION_CALL_DEPTH_TRACKING)) {
+ pr_err("WARNING: kernel not compiled with MITIGATION_CALL_DEPTH_TRACKING.\n");
+ retbleed_mitigation = RETBLEED_MITIGATION_AUTO;
}
break;
+ default:
+ break;
+ }
-do_cmd_auto:
- case RETBLEED_CMD_AUTO:
+ if (retbleed_mitigation == RETBLEED_MITIGATION_AUTO) {
if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD ||
boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
if (IS_ENABLED(CONFIG_MITIGATION_UNRET_ENTRY))
@@ -1192,17 +1167,57 @@ static void __init retbleed_select_mitigation(void)
boot_cpu_has(X86_FEATURE_IBPB))
retbleed_mitigation = RETBLEED_MITIGATION_IBPB;
}
+ }
+}
- /*
- * The Intel mitigation (IBRS or eIBRS) was already selected in
- * spectre_v2_select_mitigation(). 'retbleed_mitigation' will
- * be set accordingly below.
- */
+static void __init retbleed_update_mitigation(void)
+{
+ if (!boot_cpu_has_bug(X86_BUG_RETBLEED) || cpu_mitigations_off())
+ return;
- break;
+ if (retbleed_mitigation == RETBLEED_MITIGATION_NONE)
+ goto out;
+ /*
+ * Let IBRS trump all on Intel without affecting the effects of the
+ * retbleed= cmdline option except for call depth based stuffing
+ */
+ if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
+ switch (spectre_v2_enabled) {
+ case SPECTRE_V2_IBRS:
+ retbleed_mitigation = RETBLEED_MITIGATION_IBRS;
+ break;
+ case SPECTRE_V2_EIBRS:
+ case SPECTRE_V2_EIBRS_RETPOLINE:
+ case SPECTRE_V2_EIBRS_LFENCE:
+ retbleed_mitigation = RETBLEED_MITIGATION_EIBRS;
+ break;
+ default:
+ if (retbleed_mitigation != RETBLEED_MITIGATION_STUFF)
+ pr_err(RETBLEED_INTEL_MSG);
+ }
}
+ if (retbleed_mitigation == RETBLEED_MITIGATION_STUFF) {
+ if (spectre_v2_enabled != SPECTRE_V2_RETPOLINE) {
+ pr_err("WARNING: retbleed=stuff depends on spectre_v2=retpoline\n");
+ retbleed_mitigation = RETBLEED_MITIGATION_AUTO;
+ /* Try again */
+ retbleed_select_mitigation();
+ }
+ }
+out:
+ pr_info("%s\n", retbleed_strings[retbleed_mitigation]);
+}
+
+
+static void __init retbleed_apply_mitigation(void)
+{
+ bool mitigate_smt = false;
+
switch (retbleed_mitigation) {
+ case RETBLEED_MITIGATION_NONE:
+ return;
+
case RETBLEED_MITIGATION_UNRET:
setup_force_cpu_cap(X86_FEATURE_RETHUNK);
setup_force_cpu_cap(X86_FEATURE_UNRET);
@@ -1254,27 +1269,6 @@ static void __init retbleed_select_mitigation(void)
(retbleed_nosmt || cpu_mitigations_auto_nosmt()))
cpu_smt_disable(false);
- /*
- * Let IBRS trump all on Intel without affecting the effects of the
- * retbleed= cmdline option except for call depth based stuffing
- */
- if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
- switch (spectre_v2_enabled) {
- case SPECTRE_V2_IBRS:
- retbleed_mitigation = RETBLEED_MITIGATION_IBRS;
- break;
- case SPECTRE_V2_EIBRS:
- case SPECTRE_V2_EIBRS_RETPOLINE:
- case SPECTRE_V2_EIBRS_LFENCE:
- retbleed_mitigation = RETBLEED_MITIGATION_EIBRS;
- break;
- default:
- if (retbleed_mitigation != RETBLEED_MITIGATION_STUFF)
- pr_err(RETBLEED_INTEL_MSG);
- }
- }
-
- pr_info("%s\n", retbleed_strings[retbleed_mitigation]);
}
#undef pr_fmt
@@ -1827,16 +1821,6 @@ static void __init spectre_v2_select_mitigation(void)
break;
}
- if (IS_ENABLED(CONFIG_MITIGATION_IBRS_ENTRY) &&
- boot_cpu_has_bug(X86_BUG_RETBLEED) &&
- retbleed_cmd != RETBLEED_CMD_OFF &&
- retbleed_cmd != RETBLEED_CMD_STUFF &&
- boot_cpu_has(X86_FEATURE_IBRS) &&
- boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
- mode = SPECTRE_V2_IBRS;
- break;
- }
-
mode = spectre_v2_select_retpoline();
break;
@@ -1979,7 +1963,7 @@ static void __init spectre_v2_select_mitigation(void)
(boot_cpu_data.x86_vendor == X86_VENDOR_AMD ||
boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)) {
- if (retbleed_cmd != RETBLEED_CMD_IBPB) {
+ if (retbleed_mitigation != RETBLEED_MITIGATION_IBPB) {
setup_force_cpu_cap(X86_FEATURE_USE_IBPB_FW);
pr_info("Enabling Speculation Barrier for firmware calls\n");
}
--
2.34.1On Wed, Jan 08, 2025 at 02:24:52PM -0600, David Kaplan wrote:
> +static void __init retbleed_update_mitigation(void)
> +{
> + if (!boot_cpu_has_bug(X86_BUG_RETBLEED) || cpu_mitigations_off())
> + return;
>
> - break;
> + if (retbleed_mitigation == RETBLEED_MITIGATION_NONE)
> + goto out;
> + /*
> + * Let IBRS trump all on Intel without affecting the effects of the
> + * retbleed= cmdline option except for call depth based stuffing
> + */
> + if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
> + switch (spectre_v2_enabled) {
> + case SPECTRE_V2_IBRS:
> + retbleed_mitigation = RETBLEED_MITIGATION_IBRS;
> + break;
> + case SPECTRE_V2_EIBRS:
> + case SPECTRE_V2_EIBRS_RETPOLINE:
> + case SPECTRE_V2_EIBRS_LFENCE:
> + retbleed_mitigation = RETBLEED_MITIGATION_EIBRS;
> + break;
> + default:
> + if (retbleed_mitigation != RETBLEED_MITIGATION_STUFF)
> + pr_err(RETBLEED_INTEL_MSG);
> + }
> }
>
> + if (retbleed_mitigation == RETBLEED_MITIGATION_STUFF) {
> + if (spectre_v2_enabled != SPECTRE_V2_RETPOLINE) {
> + pr_err("WARNING: retbleed=stuff depends on spectre_v2=retpoline\n");
> + retbleed_mitigation = RETBLEED_MITIGATION_AUTO;
> + /* Try again */
> + retbleed_select_mitigation();
Err, why?
spectre_v2 and spectre_v2_enabled cannot change anymore - the select function
has set them. Why try again here?
This kinda defeats the whole purpose of having the select -> update -> apply
rounds...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Wed, 8 Jan 2025 at 21:29, David Kaplan <david.kaplan@amd.com> wrote:
> @@ -1827,16 +1821,6 @@ static void __init spectre_v2_select_mitigation(void)
> break;
> }
>
> - if (IS_ENABLED(CONFIG_MITIGATION_IBRS_ENTRY) &&
> - boot_cpu_has_bug(X86_BUG_RETBLEED) &&
> - retbleed_cmd != RETBLEED_CMD_OFF &&
> - retbleed_cmd != RETBLEED_CMD_STUFF &&
> - boot_cpu_has(X86_FEATURE_IBRS) &&
> - boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
> - mode = SPECTRE_V2_IBRS;
> - break;
> - }
> -
> mode = spectre_v2_select_retpoline();
> break;
It isn't quite clear why this gets removed here. Doesn't
retbleed_update_mitigation() still depend on this?
It gets added back in 15/35 so this would be at most a problem of git history.
On Wed, Jan 08, 2025 at 02:24:52PM -0600, David Kaplan wrote:
> @@ -1254,27 +1269,6 @@ static void __init retbleed_select_mitigation(void)
> (retbleed_nosmt || cpu_mitigations_auto_nosmt()))
> cpu_smt_disable(false);
>
> - /*
> - * Let IBRS trump all on Intel without affecting the effects of the
> - * retbleed= cmdline option except for call depth based stuffing
> - */
> - if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
> - switch (spectre_v2_enabled) {
> - case SPECTRE_V2_IBRS:
> - retbleed_mitigation = RETBLEED_MITIGATION_IBRS;
> - break;
> - case SPECTRE_V2_EIBRS:
> - case SPECTRE_V2_EIBRS_RETPOLINE:
> - case SPECTRE_V2_EIBRS_LFENCE:
> - retbleed_mitigation = RETBLEED_MITIGATION_EIBRS;
> - break;
> - default:
> - if (retbleed_mitigation != RETBLEED_MITIGATION_STUFF)
> - pr_err(RETBLEED_INTEL_MSG);
> - }
> - }
> -
> - pr_info("%s\n", retbleed_strings[retbleed_mitigation]);
> }
Extra whitespace at end of function.
--
Josh
On Wed, Jan 08, 2025 at 02:24:52PM -0600, David Kaplan wrote:
[...]
> @@ -1064,6 +1063,7 @@ enum spectre_v2_mitigation spectre_v2_enabled __ro_after_init = SPECTRE_V2_NONE;
>
> enum retbleed_mitigation {
> RETBLEED_MITIGATION_NONE,
> + RETBLEED_MITIGATION_AUTO,
This new enum ...
> RETBLEED_MITIGATION_UNRET,
> RETBLEED_MITIGATION_IBPB,
> RETBLEED_MITIGATION_IBRS,
> @@ -1071,14 +1071,6 @@ enum retbleed_mitigation {
> RETBLEED_MITIGATION_STUFF,
> };
>
> -enum retbleed_mitigation_cmd {
> - RETBLEED_CMD_OFF,
> - RETBLEED_CMD_AUTO,
> - RETBLEED_CMD_UNRET,
> - RETBLEED_CMD_IBPB,
> - RETBLEED_CMD_STUFF,
> -};
> -
> static const char * const retbleed_strings[] = {
> [RETBLEED_MITIGATION_NONE] = "Vulnerable",
> [RETBLEED_MITIGATION_UNRET] = "Mitigation: untrained return thunk",
... does not have a corresponding entry in the strings array. AUTO is the
default, and it is possible that mitigation mode can stay AUTO throughout
the retbleed mitigation selection depending on cmdline and CONFIGs. e.g.
retbleed=stuff and spectre_v2=off.
Other issue is below print in retbleed_update_mitigation() will dereference
a NULL pointer:
pr_info("%s\n", retbleed_strings[retbleed_mitigation]);
On Wed, 8 Jan 2025 21:22:37 -0800
Pawan Gupta <pawan.kumar.gupta@linux.intel.com> wrote:
> On Wed, Jan 08, 2025 at 02:24:52PM -0600, David Kaplan wrote:
> [...]
> > @@ -1064,6 +1063,7 @@ enum spectre_v2_mitigation spectre_v2_enabled __ro_after_init = SPECTRE_V2_NONE;
> >
> > enum retbleed_mitigation {
> > RETBLEED_MITIGATION_NONE,
> > + RETBLEED_MITIGATION_AUTO,
>
> This new enum ...
>
> > RETBLEED_MITIGATION_UNRET,
> > RETBLEED_MITIGATION_IBPB,
> > RETBLEED_MITIGATION_IBRS,
> > @@ -1071,14 +1071,6 @@ enum retbleed_mitigation {
> > RETBLEED_MITIGATION_STUFF,
> > };
...
> > static const char * const retbleed_strings[] = {
> > [RETBLEED_MITIGATION_NONE] = "Vulnerable",
> > [RETBLEED_MITIGATION_UNRET] = "Mitigation: untrained return thunk",
>
> ... does not have a corresponding entry in the strings array. AUTO is the
> default, and it is possible that mitigation mode can stay AUTO throughout
> the retbleed mitigation selection depending on cmdline and CONFIGs. e.g.
> retbleed=stuff and spectre_v2=off.
It is possible to use 'a bit of cpp magic' to put the definitions on one line.
Something like:
#define RETBLEED_MITIGATION(x) \
x(NONE, "Vulnerable") \
x(AUTO, "xxxx") \
x(UNRET, "Mitigation: untrained return thunk") \
...
#define X(NAME, msg) RETBLEED_MITIGATION_##NAME),
enum retbleed_mitigation { RETBLEED_MITIGATION(X) };
#undef X
#define X(NAME, msg) [RETBLEED_MITIGATION_##NAME] = msg,
static const char * const retbleed_strings[] = { RETBLEED_MITIGATION(X) };
#undef X
Then you can't lose message texts even when they are in a different file.
The lower case name (for the strcmp() loop) can also be added.
(and don't let the rust bindgen near it :-)
David
On Fri, Jan 10, 2025 at 06:45:45PM +0000, David Laight wrote:
> On Wed, 8 Jan 2025 21:22:37 -0800
> Pawan Gupta <pawan.kumar.gupta@linux.intel.com> wrote:
>
> > On Wed, Jan 08, 2025 at 02:24:52PM -0600, David Kaplan wrote:
> > [...]
> > > @@ -1064,6 +1063,7 @@ enum spectre_v2_mitigation spectre_v2_enabled __ro_after_init = SPECTRE_V2_NONE;
> > >
> > > enum retbleed_mitigation {
> > > RETBLEED_MITIGATION_NONE,
> > > + RETBLEED_MITIGATION_AUTO,
> >
> > This new enum ...
> >
> > > RETBLEED_MITIGATION_UNRET,
> > > RETBLEED_MITIGATION_IBPB,
> > > RETBLEED_MITIGATION_IBRS,
> > > @@ -1071,14 +1071,6 @@ enum retbleed_mitigation {
> > > RETBLEED_MITIGATION_STUFF,
> > > };
> ...
> > > static const char * const retbleed_strings[] = {
> > > [RETBLEED_MITIGATION_NONE] = "Vulnerable",
> > > [RETBLEED_MITIGATION_UNRET] = "Mitigation: untrained return thunk",
> >
> > ... does not have a corresponding entry in the strings array. AUTO is the
> > default, and it is possible that mitigation mode can stay AUTO throughout
> > the retbleed mitigation selection depending on cmdline and CONFIGs. e.g.
> > retbleed=stuff and spectre_v2=off.
>
> It is possible to use 'a bit of cpp magic' to put the definitions on one line.
> Something like:
> #define RETBLEED_MITIGATION(x) \
> x(NONE, "Vulnerable") \
> x(AUTO, "xxxx") \
> x(UNRET, "Mitigation: untrained return thunk") \
> ...
>
> #define X(NAME, msg) RETBLEED_MITIGATION_##NAME),
> enum retbleed_mitigation { RETBLEED_MITIGATION(X) };
> #undef X
>
> #define X(NAME, msg) [RETBLEED_MITIGATION_##NAME] = msg,
> static const char * const retbleed_strings[] = { RETBLEED_MITIGATION(X) };
> #undef X
>
> Then you can't lose message texts even when they are in a different file.
> The lower case name (for the strcmp() loop) can also be added.
>
> (and don't let the rust bindgen near it :-)
Wow, this is mind blowing!
On Fri, Jan 10, 2025 at 12:30:58PM -0800, Pawan Gupta wrote:
> Wow, this is mind blowing!
Yeah, no, we'll never do that, no worries.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
© 2016 - 2025 Red Hat, Inc.