2022-03-03 14:48:46

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH v3 25/39] x86/bugs: Disable Retpoline when IBT

Retpoline and IBT are mutually exclusive. IBT relies on indirect
branches (JMP/CALL *%reg) while retpoline avoids them by design.

Demote to LFENCE on IBT enabled hardware.

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
arch/x86/kernel/cpu/bugs.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -891,6 +891,7 @@ static void __init spectre_v2_select_mit
{
enum spectre_v2_mitigation_cmd cmd = spectre_v2_parse_cmdline();
enum spectre_v2_mitigation mode = SPECTRE_V2_NONE;
+ bool silent_demote = false;

/*
* If the CPU is not affected and the command line mode is NONE or AUTO
@@ -906,6 +907,7 @@ static void __init spectre_v2_select_mit

case SPECTRE_V2_CMD_FORCE:
case SPECTRE_V2_CMD_AUTO:
+ silent_demote = true;
if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
mode = SPECTRE_V2_IBRS_ENHANCED;
/* Force it so VMEXIT will restore correctly */
@@ -938,6 +940,7 @@ static void __init spectre_v2_select_mit
retpoline_amd:
if (!boot_cpu_has(X86_FEATURE_LFENCE_RDTSC)) {
pr_err("Spectre mitigation: LFENCE not serializing, switching to generic retpoline\n");
+ silent_demote = false;
goto retpoline_generic;
}
mode = SPECTRE_V2_RETPOLINE_AMD;
@@ -947,6 +950,16 @@ static void __init spectre_v2_select_mit
retpoline_generic:
mode = SPECTRE_V2_RETPOLINE_GENERIC;
setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
+
+ /*
+ * ROP defeats IBT, make sure not to use Retpolines and IBT together.
+ */
+ if (HAS_KERNEL_IBT && cpu_feature_enabled(X86_FEATURE_IBT)) {
+ if (!silent_demote)
+ pr_warn("Spectre mitigation: Switching to LFENCE due to IBT\n");
+ mode = SPECTRE_V2_RETPOLINE_AMD;
+ setup_force_cpu_cap(X86_FEATURE_RETPOLINE_AMD);
+ }
}

specv2_set_mode:



2022-03-04 19:58:25

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH v3 25/39] x86/bugs: Disable Retpoline when IBT

On Thu, Mar 03, 2022 at 12:23:46PM +0100, Peter Zijlstra wrote:
> Retpoline and IBT are mutually exclusive. IBT relies on indirect
> branches (JMP/CALL *%reg) while retpoline avoids them by design.
>
> Demote to LFENCE on IBT enabled hardware.
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> arch/x86/kernel/cpu/bugs.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -891,6 +891,7 @@ static void __init spectre_v2_select_mit
> {
> enum spectre_v2_mitigation_cmd cmd = spectre_v2_parse_cmdline();
> enum spectre_v2_mitigation mode = SPECTRE_V2_NONE;
> + bool silent_demote = false;
>
> /*
> * If the CPU is not affected and the command line mode is NONE or AUTO
> @@ -906,6 +907,7 @@ static void __init spectre_v2_select_mit
>
> case SPECTRE_V2_CMD_FORCE:
> case SPECTRE_V2_CMD_AUTO:
> + silent_demote = true;
> if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> mode = SPECTRE_V2_IBRS_ENHANCED;
> /* Force it so VMEXIT will restore correctly */
> @@ -938,6 +940,7 @@ static void __init spectre_v2_select_mit
> retpoline_amd:
> if (!boot_cpu_has(X86_FEATURE_LFENCE_RDTSC)) {
> pr_err("Spectre mitigation: LFENCE not serializing, switching to generic retpoline\n");
> + silent_demote = false;
> goto retpoline_generic;
> }
> mode = SPECTRE_V2_RETPOLINE_AMD;
> @@ -947,6 +950,16 @@ static void __init spectre_v2_select_mit
> retpoline_generic:
> mode = SPECTRE_V2_RETPOLINE_GENERIC;
> setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
> +
> + /*
> + * ROP defeats IBT, make sure not to use Retpolines and IBT together.
> + */
> + if (HAS_KERNEL_IBT && cpu_feature_enabled(X86_FEATURE_IBT)) {
> + if (!silent_demote)
> + pr_warn("Spectre mitigation: Switching to LFENCE due to IBT\n");
> + mode = SPECTRE_V2_RETPOLINE_AMD;
> + setup_force_cpu_cap(X86_FEATURE_RETPOLINE_AMD);
> + }

This is better. But AFAIK, the 'silent_demote' case should only happen
on a hypothetically weird/broken virt setup, right? Why silence the
warning? It still seems legit. If you have IBT on non-eIBRS Intel, and
you get silently demoted from retpoline to lfence, it presumably opens
up some Spectre v2 attack vectors, despite IBT's implicit promise of
providing *more* protection overall.

And actually, after thinking about it, my most preferred approach would
be to do the converse of this patch: only enable IBT if
!X86_FEATURE_RETPOLINE. And do a real warning, like "your setup is
broken, IBT is disabled". Maybe even make it a real WARN() so we could
find out if it's a real possibility. Since this feature is much newer
than retpoline, that would probably be the simplest and least surprising
option.

--
Josh