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

[Andi Kleen]

> +/*
> + * 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
 }