Messages in this thread Patch in this message | | | Date | Wed, 13 Oct 2021 23:20:02 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 4/9] x86/alternative: Implement .retpoline_sites support |
| |
On Wed, Oct 13, 2021 at 01:39:27PM -0700, Josh Poimboeuf wrote: > On Wed, Oct 13, 2021 at 02:22:21PM +0200, Peter Zijlstra wrote: > > +static int patch_retpoline(void *addr, struct insn *insn, u8 *bytes) > > +{ > > + void (*target)(void); > > + int reg, i = 0; > > + > > + if (cpu_feature_enabled(X86_FEATURE_RETPOLINE)) > > + return -1; > > Better to do this check further up the call stack in apply_retpolines() > before looping through all the call sites?
In fact, I've pushed it further down, in order to always validate the absense of rsp.
> > + > > + target = addr + insn->length + insn->immediate.value; > > + reg = (target - &__x86_indirect_thunk_rax) / > > + (&__x86_indirect_thunk_rcx - &__x86_indirect_thunk_rax); > > + > > + if (WARN_ON_ONCE(reg & ~0xf)) > > + return -1; > > It would be more robust and less magical to just have a basic lookup > table array which converts a thunk address to a reg. Then you can just > avoid all the safety checks because it's no longer insane ;-)
Andrew suggested the reverse lookup to validate. That should give the same robustness but lacks the linear lookup.
--- --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -392,6 +392,12 @@ static int emit_indirect(int op, int reg */ static int patch_retpoline(void *addr, struct insn *insn, u8 *bytes) { + static const void *reg_to_thunk[] = { +#undef GEN +#define GEN(reg) &__x86_indirect_thunk_ ## reg, +#include <asm/GEN-for-each-reg.h> + }; + void (*target)(void); int reg, i = 0; @@ -402,6 +408,8 @@ static int patch_retpoline(void *addr, s if (WARN_ON_ONCE(reg & ~0xf)) return -1; + BUG_ON(target != reg_to_thunk[reg]); + /* * If anyone ever does: CALL/JMP *%rsp, we're in deep trouble. */
| |