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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 11 Jan 2018 15:51:17 -0800
From:   Andi Kleen <ak@...ux.intel.com>
To:     David Woodhouse <dwmw@...zon.co.uk>
Cc:     Paul Turner <pjt@...gle.com>, LKML <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...ux-foundation.org>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Dave Hansen <dave.hansen@...el.com>, tglx@...utronix.de,
        Kees Cook <keescook@...gle.com>,
        Rik van Riel <riel@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Andy Lutomirski <luto@...capital.net>,
        Jiri Kosina <jikos@...nel.org>, gnomes@...rguk.ukuu.org.uk,
        x86@...nel.org, thomas.lendacky@....com,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [PATCH v8 12/12] x86/retpoline: Fill return stack buffer on
 vmexit

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