Messages in this thread | | | Date | Fri, 4 Mar 2022 09:39:39 -0800 | From | Josh Poimboeuf <> | 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) <peterz@infradead.org> > --- > 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
| |