Re: [PATCH v8 12/12] x86/retpoline: Fill return stack buffer on vmexit

From: Andi Kleen
Date: Thu Jan 11 2018 - 18:52:03 EST


> +/*
> + * On VMEXIT we must ensure that no RSB predictions learned in the guest
> + * can be followed in the host, by overwriting the RSB completely. Both
> + * retpoline and IBRS mitigations for Spectre v2 need this; only on future
> + * CPUs with IBRS_ATT *might* it be avoided.
> + */
> +static inline void vmexit_fill_RSB(void)
> +{
> +#ifdef CONFIG_RETPOLINE
> + unsigned long loops = RSB_CLEAR_LOOPS / 2;
> +
> + asm volatile (ANNOTATE_NOSPEC_ALTERNATIVE
> + ALTERNATIVE("jmp 910f",
> + __stringify(__FILL_RETURN_BUFFER(%0, RSB_CLEAR_LOOPS, %1, __LINE__)),

This __LINE__ thing is broken. The expanded assembler doesn't use the number,
but ends up with a label with __LINE__, and if you actually use the inline
multiple times in a file you end up with

/home/ak/lsrc/git/linux/arch/x86/include/asm/nospec-branch.h:241: Error: symbol `.Ldo_call1___LINE__' is already defined
/home/ak/lsrc/git/linux/arch/x86/include/asm/nospec-branch.h:241: Error: symbol `.Ltrap1___LINE__' is already defined
/home/ak/lsrc/git/linux/arch/x86/include/asm/nospec-branch.h:241: Error: symbol `.Ldo_call2___LINE__' is already defined
/home/ak/lsrc/git/linux/arch/x86/include/asm/nospec-branch.h:241: Error: symbol `.Ltrap2___LINE__' is already defined
/home/ak/lsrc/git/linux/arch/x86/include/asm/nospec-branch.h:241: Error: symbol `.Ldo_loop___LINE__' is already defined

I used this incremential patch, which seems to fix that:


diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 475ab0cb80c0..53ea99bf5ed5 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -27,21 +27,21 @@
#define RSB_CLEAR_LOOPS 32 /* To forcibly overwrite all entries */
#define RSB_FILL_LOOPS 16 /* To avoid underflow */

-#define __FILL_RETURN_BUFFER(reg, nr, sp, uniq) \
+#define __FILL_RETURN_BUFFER(reg, nr, sp) \
mov $(nr/2), reg; \
-.Ldo_call1_ ## uniq: \
- call .Ldo_call2_ ## uniq; \
-.Ltrap1_ ## uniq: \
+771: \
+ call 772f; \
+773: \
pause; \
- jmp .Ltrap1_ ## uniq; \
-.Ldo_call2_ ## uniq: \
- call .Ldo_loop_ ## uniq; \
-.Ltrap2_ ## uniq: \
+ jmp 773b; \
+772: \
+ call 774f; \
+775: \
pause; \
- jmp .Ltrap2_ ## uniq; \
-.Ldo_loop_ ## uniq: \
+ jmp 775b; \
+774: \
dec reg; \
- jnz .Ldo_call1_ ## uniq; \
+ jnz 771b; \
add $(BITS_PER_LONG/8) * nr, sp;

#ifdef __ASSEMBLY__
@@ -121,7 +121,7 @@
#ifdef CONFIG_RETPOLINE
ANNOTATE_NOSPEC_ALTERNATIVE
ALTERNATIVE "jmp .Lskip_rsb_\@", \
- __stringify(__FILL_RETURN_BUFFER(\reg,\nr,%_ASM_SP,\@)) \
+ __stringify(__FILL_RETURN_BUFFER(\reg,\nr,%_ASM_SP)) \
\ftr
.Lskip_rsb_\@:
#endif
@@ -198,10 +198,10 @@ static inline void vmexit_fill_RSB(void)

asm volatile (ANNOTATE_NOSPEC_ALTERNATIVE
ALTERNATIVE("jmp 910f",
- __stringify(__FILL_RETURN_BUFFER(%0, RSB_CLEAR_LOOPS, %1, __LINE__)),
+ __stringify(__FILL_RETURN_BUFFER(%0, RSB_CLEAR_LOOPS, %1)),
X86_FEATURE_RETPOLINE)
"910:"
: "=&r" (loops), ASM_CALL_CONSTRAINT
: "r" (loops) : "memory" );
#endif
}