Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 11 Apr 2024 17:15:22 -0700 | From | Pawan Gupta <> | Subject | Re: [PATCH 5/7] x86/bugs: Only harden syscalls when needed |
| |
On Wed, Apr 10, 2024 at 10:40:49PM -0700, Josh Poimboeuf wrote: > Syscall hardening (i.e., converting the syscall indirect branch to a > series of direct branches) may cause performance regressions in certain > scenarios. Only use the syscall hardening when indirect branches are > considered unsafe. > > Fixes: 1e3ad78334a6 ("x86/syscall: Don't force use of indirect calls for system calls") > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> > --- > arch/x86/entry/common.c | 30 +++++++++++++++++++++++++--- > arch/x86/entry/syscall_32.c | 11 +--------- > arch/x86/entry/syscall_64.c | 8 +------- > arch/x86/entry/syscall_x32.c | 7 ++++++- > arch/x86/include/asm/cpufeatures.h | 1 + > arch/x86/include/asm/syscall.h | 8 +++++++- > arch/x86/kernel/cpu/bugs.c | 32 +++++++++++++++++++++++++++++- > 7 files changed, 74 insertions(+), 23 deletions(-) > > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c > index 6de50b80702e..80d432d2fe44 100644 > --- a/arch/x86/entry/common.c > +++ b/arch/x86/entry/common.c > @@ -39,6 +39,28 @@ > > #ifdef CONFIG_X86_64 > > +/* > + * Do either a direct or an indirect call, depending on whether indirect calls > + * are considered safe. > + */ > +#define __do_syscall(table, func_direct, nr, regs) \ > +({ \ > + unsigned long __rax, __rdi, __rsi; \ > + \ > + asm_inline volatile( \ > + ALTERNATIVE("call " __stringify(func_direct) "\n\t", \ > + ANNOTATE_RETPOLINE_SAFE \ > + "call *%[func_ptr]\n\t", \
This will likely not insert the lfence before the indirect call in spectre_v2=eibrs,lfence mode. As X86_FEATURE_INDIRECT_SAFE is not cleared when eIBRS is enabled, this will not be converted to direct call.
[...] > @@ -1720,6 +1744,7 @@ static void __init spectre_v2_select_mitigation(void) > > case SPECTRE_V2_CMD_RETPOLINE_LFENCE: > pr_err(SPECTRE_V2_LFENCE_MSG); > + setup_clear_cpu_cap(X86_FEATURE_INDIRECT_SAFE);
I don't know if it intentional, this seems to be the duplicate of X86_FEATURE_INDIRECT_SAFE clear later in SPECTRE_V2_LFENCE mode. Also it seems a bit odd to do this here in SPECTRE_V2_CMD handling.
> mode = SPECTRE_V2_LFENCE; > break; > > @@ -1772,11 +1797,16 @@ static void __init spectre_v2_select_mitigation(void) > break; > > case SPECTRE_V2_LFENCE: > + setup_clear_cpu_cap(X86_FEATURE_INDIRECT_SAFE); > + fallthrough; > case SPECTRE_V2_EIBRS_LFENCE: > setup_force_cpu_cap(X86_FEATURE_RETPOLINE_LFENCE); > - fallthrough; > + setup_force_cpu_cap(X86_FEATURE_RETPOLINE); > + break; > > case SPECTRE_V2_RETPOLINE: > + setup_clear_cpu_cap(X86_FEATURE_INDIRECT_SAFE); > + fallthrough; > case SPECTRE_V2_EIBRS_RETPOLINE: > setup_force_cpu_cap(X86_FEATURE_RETPOLINE); > break;
| ![\](/images/icornerr.gif) |