On Thu, Jan 11, 2018 at 11:37:18AM +0000, David Woodhouse wrote: > In accordance with the Intel and AMD documentation, we need to overwrite > all entries in the RSB on exiting a guest, to prevent malicious branch > target predictions from affecting the host kernel. This is needed both > for retpoline and for IBRS. > > Signed-off-by: David Woodhouse <[email protected]> > --- > v2: Reduce the size of the ALTERNATIVE insns, fix .align (again)! > Sent in private email for testing, hence this second public post is > v2.1: Add CONFIG_RETPOLINE around RSB stuffing > > Sorry, Josh, objtool broke again! :) > > arch/x86/include/asm/nospec-branch.h | 76 > ++++++++++++++++++++++++++++++++++++ > arch/x86/kvm/svm.c | 4 ++ > arch/x86/kvm/vmx.c | 4 ++ > 3 files changed, 84 insertions(+) > > diff --git a/arch/x86/include/asm/nospec-branch.h > b/arch/x86/include/asm/nospec-branch.h > index 7d70ea9..b55ff79 100644 > --- a/arch/x86/include/asm/nospec-branch.h > +++ b/arch/x86/include/asm/nospec-branch.h > @@ -7,6 +7,50 @@ > #include <asm/alternative-asm.h> > #include <asm/cpufeatures.h> > > +/* > + * Fill the CPU return stack buffer. > + * > + * Each entry in the RSB, if used for a speculative 'ret', contains an > + * infinite 'pause; jmp' loop to capture speculative execution. > + * > + * This is required in various cases for retpoline and IBRS-based > + * mitigations for the Spectre variant 2 vulnerability. Sometimes to > + * eliminate potentially bogus entries from the RSB, and sometimes > + * purely to ensure that it doesn't get empty, which on some CPUs would > + * allow predictions from other (unwanted!) sources to be used. > + * > + * We define a CPP macro such that it can be used from both .S files and > + * inline assembly. It's possible to do a .macro and then include that > + * from C via asm(".include <asm/nospec-branch.h>") but let's not go there. > + */ > + > +#define RSB_CLEAR_LOOPS 32 /* To forcibly overwrite all > entries */ > +#define RSB_FILL_LOOPS 16 /* To avoid underflow */ > + > +#define __FILL_RETURN_BUFFER_PREP(reg, nr) \ > + mov $(nr/2), reg > + > +#define __FILL_RETURN_BUFFER(reg, nr, sp, uniq) \ > + .align 16; \ > +.Ldo_call1_ ## uniq: \ > + call .Ldo_call2_ ## uniq; \ > +.Ltrap1_ ## uniq: \ > + pause; \ > + jmp .Ltrap1_ ## uniq; \ > + .align 16; \ > +.Ldo_call2_ ## uniq: \ > + call .Ldo_loop_ ## uniq; \ > +.Ltrap2_ ## uniq: \ > + pause; \ > + jmp .Ltrap2_ ## uniq; \ > + .align 16; \ > +.Ldo_loop_ ## uniq: \ > + dec reg; \ > + jnz .Ldo_call1_ ## uniq; \ > + add $(BITS_PER_LONG/8) * nr, sp; \ > +.Lskip_rsb_ ## uniq:; > + > + > #ifdef __ASSEMBLY__ > > /* > @@ -61,6 +105,19 @@ > #endif > .endm > > + /* > + * A simpler FILL_RETURN_BUFFER macro. Don't make people use the CPP > + * monstrosity above, manually. > + */ > +.macro FILL_RETURN_BUFFER reg:req nr:req ftr:req > +#ifdef CONFIG_RETPOLINE > + ALTERNATIVE "jmp .Lskip_rsb_\@", \ > + __stringify(__FILL_RETURN_BUFFER_PREP(\reg,\nr)), \ > + \ftr > + __FILL_RETURN_BUFFER(\reg,\nr,%_ASM_SP,\@) > +#endif > +.endm
This seems weird. I liked v1 a lot better. What's the problem with patching in the whole thing? Also, if you go back to v1, it should be an easy objtool fix, just add ANNOTATE_NOSPEC_ALTERNATIVE in front of it. -- Josh
