Messages in this thread | | | From | Andrew Cooper <> | Subject | Re: [PATCH 4/9] x86/alternative: Implement .retpoline_sites support | Date | Wed, 13 Oct 2021 18:11:11 +0100 |
| |
On 13/10/2021 16:12, Peter Zijlstra wrote: > On Wed, Oct 13, 2021 at 03:38:27PM +0100, Andrew Cooper wrote: >> On 13/10/2021 13:22, Peter Zijlstra wrote: >>> +/* >>> + * Rewrite the compiler generated retpoline thunk calls. >>> + * >>> + * For spectre_v2=off (!X86_FEATURE_RETPOLINE), rewrite them into immediate >>> + * indirect instructions, avoiding the extra indirection. >>> + * >>> + * For example, convert: >>> + * >>> + * CALL __x86_indirect_thunk_\reg >>> + * >>> + * into: >>> + * >>> + * CALL *%\reg >>> + * >>> + */ >>> +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; >>> + >>> + target = addr + insn->length + insn->immediate.value; >>> + reg = (target - &__x86_indirect_thunk_rax) / >>> + (&__x86_indirect_thunk_rcx - &__x86_indirect_thunk_rax); >> This is equal measures beautiful and terrifying. > Thanks! :-) > >> Something around here really wants to BUG_ON(reg == 4), because >> literally nothing good can come from selecting %rsp. > Ack, I had to add rsp to get the offsets right, but indeed, if anything > ever selects that we're in trouble.
Actually, all you need is space for the RSP thunk, not an actual RSP thunk, and it's probably a wise move not to write one out.
You can fill it with 0xcc's, and make sure not to make it an exported symbol.
> >> Also, it might be a good idea (previous patch perhaps) to have some >> linker assertions to confirm that the symbols are laid out safely to do >> this calculation. > I was hoping that since all this is in .S it would be immune from crazy > things like a compiler and do as told. But I suppose carzy stuff like > LTO (or worse BOLT) can totaly wreck this still (then BOLT won't care > about linker script assertions either). > > I'll see if I can come up with something.
Another cross check could be something like:
unsigned long reg_to_thunk[] = { &__x86_indirec_thunk_rax, ... };
because then BUG_ON(target != reg_to_thunk[reg]) will catch any errors from layout issues.
Using 0 for rsp could then subsume the individual check.
~Andrew
| |