[v2] retpoline: add clang support + Kconfig selectable

[PATCH v2 3/3] x86/Kconfig: introduce option to select retpoline usage

Posted by Roger Pau Monne 3 years, 11 months ago

Add a new Kconfig option under the "Speculative hardening" section
that allows selecting whether to enable retpoline. This depends on the
underlying compiler having retpoline support.

Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
 - Fix description of option to use indirect branches instead of
   indirect calls.
---
 xen/arch/x86/Kconfig |  4 ----
 xen/arch/x86/arch.mk |  2 ++
 xen/common/Kconfig   | 16 ++++++++++++++++
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 2fa456292b..7c73802adc 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -38,10 +38,6 @@ config GCC_INDIRECT_THUNK
 config CLANG_INDIRECT_THUNK
 	def_bool $(cc-option,-mretpoline-external-thunk)
 
-config INDIRECT_THUNK
-	def_bool y
-	depends on GCC_INDIRECT_THUNK || CLANG_INDIRECT_THUNK
-
 config HAS_AS_CET_SS
 	# binutils >= 2.29 or LLVM >= 6
 	def_bool $(as-instr,wrssq %rax$(comma)0;setssbsy)
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index f2aa2a515f..0597e714f9 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -42,6 +42,7 @@ CFLAGS += -mno-red-zone -fpic
 # SSE setup for variadic function calls.
 CFLAGS += -mno-sse $(call cc-option,$(CC),-mskip-rax-setup)
 
+ifeq ($(CONFIG_INDIRECT_THUNK),y)
 # Compile with gcc thunk-extern, indirect-branch-register if available.
 CFLAGS-$(CONFIG_GCC_INDIRECT_THUNK) += -mindirect-branch=thunk-extern
 CFLAGS-$(CONFIG_GCC_INDIRECT_THUNK) += -mindirect-branch-register
@@ -49,6 +50,7 @@ CFLAGS-$(CONFIG_GCC_INDIRECT_THUNK) += -fno-jump-tables
 
 # Enable clang retpoline support if available.
 CFLAGS-$(CONFIG_CLANG_INDIRECT_THUNK) += -mretpoline-external-thunk
+endif
 
 # If supported by the compiler, reduce stack alignment to 8 bytes. But allow
 # this to be overridden elsewhere.
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index db687b1785..e688e45513 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -146,6 +146,22 @@ config SPECULATIVE_HARDEN_GUEST_ACCESS
 
 	  If unsure, say Y.
 
+config INDIRECT_THUNK
+	bool "Speculative Branch Target Injection Protection"
+	depends on X86 && (GCC_INDIRECT_THUNK || CLANG_INDIRECT_THUNK)
+	default y
+	help
+	  Contemporary processors may use speculative execution as a
+	  performance optimisation, but this can potentially be abused by an
+	  attacker to leak data via speculative sidechannels.
+
+	  One source of data leakage is via branch target injection.
+
+	  When enabled, indirect branches are implemented using a new construct
+	  called "retpoline" that prevents speculation.
+
+	  If unsure, say Y.
+
 endmenu
 
 config HYPFS
-- 
2.34.1

Re: [PATCH v2 3/3] x86/Kconfig: introduce option to select retpoline usage

Posted by Jan Beulich 3 years, 11 months ago

On 16.02.2022 17:21, Roger Pau Monne wrote:
> Add a new Kconfig option under the "Speculative hardening" section
> that allows selecting whether to enable retpoline. This depends on the
> underlying compiler having retpoline support.
> 
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

There's one aspect though which I would like to see Arm maintainer
input on:

> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -38,10 +38,6 @@ config GCC_INDIRECT_THUNK
>  config CLANG_INDIRECT_THUNK
>  	def_bool $(cc-option,-mretpoline-external-thunk)
>  
> -config INDIRECT_THUNK
> -	def_bool y
> -	depends on GCC_INDIRECT_THUNK || CLANG_INDIRECT_THUNK

Moving this ...

> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -146,6 +146,22 @@ config SPECULATIVE_HARDEN_GUEST_ACCESS
>  
>  	  If unsure, say Y.
>  
> +config INDIRECT_THUNK
> +	bool "Speculative Branch Target Injection Protection"
> +	depends on X86 && (GCC_INDIRECT_THUNK || CLANG_INDIRECT_THUNK)

... here despite being explicitly marked x86-specific looks a
little odd. Since the dependencies are x86-specific, dropping
X86 from here would make my slight concern go away.

Jan

Re: [PATCH v2 3/3] x86/Kconfig: introduce option to select retpoline usage

Posted by Roger Pau Monné 3 years, 11 months ago

On Thu, Feb 17, 2022 at 10:07:32AM +0100, Jan Beulich wrote:
> On 16.02.2022 17:21, Roger Pau Monne wrote:
> > Add a new Kconfig option under the "Speculative hardening" section
> > that allows selecting whether to enable retpoline. This depends on the
> > underlying compiler having retpoline support.
> > 
> > Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> There's one aspect though which I would like to see Arm maintainer
> input on:
> 
> > --- a/xen/arch/x86/Kconfig
> > +++ b/xen/arch/x86/Kconfig
> > @@ -38,10 +38,6 @@ config GCC_INDIRECT_THUNK
> >  config CLANG_INDIRECT_THUNK
> >  	def_bool $(cc-option,-mretpoline-external-thunk)
> >  
> > -config INDIRECT_THUNK
> > -	def_bool y
> > -	depends on GCC_INDIRECT_THUNK || CLANG_INDIRECT_THUNK
> 
> Moving this ...
> 
> > --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -146,6 +146,22 @@ config SPECULATIVE_HARDEN_GUEST_ACCESS
> >  
> >  	  If unsure, say Y.
> >  
> > +config INDIRECT_THUNK
> > +	bool "Speculative Branch Target Injection Protection"
> > +	depends on X86 && (GCC_INDIRECT_THUNK || CLANG_INDIRECT_THUNK)
> 
> ... here despite being explicitly marked x86-specific looks a
> little odd. Since the dependencies are x86-specific, dropping
> X86 from here would make my slight concern go away.

Right - I've added the X86 because I was concerned about GCC or CLANG
also exposing the repoline options on Arm, but that's not an issue
because the compiler tests are only done for x86 anyway.

Feel free to drop the 'X86 &&' and the parentheses if you wish.
Otherwise I can resend if you prefer.

Thanks, Roger.

Re: [PATCH v2 3/3] x86/Kconfig: introduce option to select retpoline usage

Posted by Jan Beulich 3 years, 11 months ago

On 17.02.2022 11:34, Roger Pau Monné wrote:
> On Thu, Feb 17, 2022 at 10:07:32AM +0100, Jan Beulich wrote:
>> On 16.02.2022 17:21, Roger Pau Monne wrote:
>>> Add a new Kconfig option under the "Speculative hardening" section
>>> that allows selecting whether to enable retpoline. This depends on the
>>> underlying compiler having retpoline support.
>>>
>>> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>
>> There's one aspect though which I would like to see Arm maintainer
>> input on:
>>
>>> --- a/xen/arch/x86/Kconfig
>>> +++ b/xen/arch/x86/Kconfig
>>> @@ -38,10 +38,6 @@ config GCC_INDIRECT_THUNK
>>>  config CLANG_INDIRECT_THUNK
>>>  	def_bool $(cc-option,-mretpoline-external-thunk)
>>>  
>>> -config INDIRECT_THUNK
>>> -	def_bool y
>>> -	depends on GCC_INDIRECT_THUNK || CLANG_INDIRECT_THUNK
>>
>> Moving this ...
>>
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -146,6 +146,22 @@ config SPECULATIVE_HARDEN_GUEST_ACCESS
>>>  
>>>  	  If unsure, say Y.
>>>  
>>> +config INDIRECT_THUNK
>>> +	bool "Speculative Branch Target Injection Protection"
>>> +	depends on X86 && (GCC_INDIRECT_THUNK || CLANG_INDIRECT_THUNK)
>>
>> ... here despite being explicitly marked x86-specific looks a
>> little odd. Since the dependencies are x86-specific, dropping
>> X86 from here would make my slight concern go away.
> 
> Right - I've added the X86 because I was concerned about GCC or CLANG
> also exposing the repoline options on Arm, but that's not an issue
> because the compiler tests are only done for x86 anyway.
> 
> Feel free to drop the 'X86 &&' and the parentheses if you wish.
> Otherwise I can resend if you prefer.

No need to resend just for this.

Jan