Messages in this thread | | | Date | Thu, 11 Apr 2024 22:27:27 -0700 | From | Pawan Gupta <> | Subject | Re: [PATCH 5/7] x86/bugs: Only harden syscalls when needed |
| |
On Thu, Apr 11, 2024 at 08:57:40PM -0700, Josh Poimboeuf wrote: > On Thu, Apr 11, 2024 at 05:15:22PM -0700, Pawan Gupta wrote: > > > + * 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. > > Hm, I think the problem here is that SPECTRE_V2_EIBRS_LFENCE confusingly > sets X86_FEATURE_RETPOLINE. So the following bit unintentionally takes
I think it is intentional, more on it below.
> effect: > > /* Retpoline mitigates against BHI unless the CPU has RRSBA behavior */ > if (cpu_feature_enabled(X86_FEATURE_RETPOLINE)) { > spec_ctrl_disable_kernel_rrsba(); > if (rrsba_disabled) > return; > } > > If RRSBA gets disabled (which is likely), bhi_select_mitigation() > returns early and X86_FEATURE_INDIRECT_SAFE doesn't get cleared. > > "LFENCE; CALL" is most definitely not a retpoline, so it's weird for > SPECTRE_V2_EIBRS_LFENCE to be setting X86_FEATURE_RETPOLINE. We should > fix that.
I could be completely wrong here, but my guess is, it is needed because the thunk call inserted by the compiler with X86_FEATURE_RETPOLINE provides room for adding the extra lfence.
In order to prefix lfence(3 bytes) indirect call is first converted to call __x86_indirect_thunk_reg, which has a 5 byte opcode. At runtime, thunk call is patched to "lfence;call *reg", which is also 3+2=5 bytes.
Thunk call is anyways needed because, there are indirect calls opcodes that are 3 byte long e.g. call *%r8. So, wherever possible lfence+call* is inlined, otherwise lfence is executed in a call to thunk, which then does jmp *%reg.
> Honestly, I think SPECTRE_V2_EIBRS_LFENCE is obsolete anyway. It was > originally intended to be a BHI mitigation, but the real-world > benchmarks I've seen are showing it to be quite a bit slower than the > actual BHI mitigations. > > Plus it's only a partial fix because the speculative window after the > branch can still be big enough to do multiple loads.
Thats fair.
| |