[PATCH 0/3] retpoline: add clang support + Kconfig selectable

[PATCH 0/3] retpoline: add clang support + Kconfig selectable

[Roger Pau Monne]

Hello,

The following series adds retpoline support for clang builds, and also
allows the user to select whether to enable retpoline support at build
time via a new Kconfig option.

I've tried adding a suitable description to the Kconfig option, but I'm
sure there's room for improvement.

Thanks, Roger.

Roger Pau Monne (3):
  x86/retpoline: rename retpoline Kconfig check to include GCC prefix
  x86/clang: add retpoline support
  x86/Kconfig: introduce option to select retpoline usage

 xen/arch/x86/Kconfig |  5 ++++-
 xen/arch/x86/arch.mk | 13 +++++++++----
 xen/common/Kconfig   | 16 ++++++++++++++++
 3 files changed, 29 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH 1/3] x86/retpoline: rename retpoline Kconfig check to include GCC prefix

[Roger Pau Monne]

Current retpoline checks apply to GCC only, so make it obvious by
prefixing the Kconfig option with GCC. Keep the previous option as a
way to signal generic retpoline support regardless of the underlying
compiler.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/Kconfig | 6 +++++-
 xen/arch/x86/arch.mk | 8 ++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index b4abfca46f..2cd713724f 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -32,9 +32,13 @@ config ARCH_DEFCONFIG
 	string
 	default "arch/x86/configs/x86_64_defconfig"
 
-config INDIRECT_THUNK
+config GCC_INDIRECT_THUNK
 	def_bool $(cc-option,-mindirect-branch-register)
 
+config INDIRECT_THUNK
+	depends on GCC_INDIRECT_THUNK
+	def_bool y
+
 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 fa7cf38443..2da4bdb1ed 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -42,10 +42,10 @@ CFLAGS += -mno-red-zone -fpic
 # SSE setup for variadic function calls.
 CFLAGS += -mno-sse $(call cc-option,$(CC),-mskip-rax-setup)
 
-# Compile with thunk-extern, indirect-branch-register if avaiable.
-CFLAGS-$(CONFIG_INDIRECT_THUNK) += -mindirect-branch=thunk-extern
-CFLAGS-$(CONFIG_INDIRECT_THUNK) += -mindirect-branch-register
-CFLAGS-$(CONFIG_INDIRECT_THUNK) += -fno-jump-tables
+# 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
+CFLAGS-$(CONFIG_GCC_INDIRECT_THUNK) += -fno-jump-tables
 
 # If supported by the compiler, reduce stack alignment to 8 bytes. But allow
 # this to be overridden elsewhere.
-- 
2.34.1

[PATCH 2/3] x86/clang: add retpoline support

[Roger Pau Monne]

Detect whether the compiler supports clang retpoline option and enable
by default if available, just like it's done for gcc.

Note clang already disables jump tables when retpoline is enabled, so
there's no need to also pass the fno-jump-tables parameter.

Reported-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/Kconfig | 5 ++++-
 xen/arch/x86/arch.mk | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 2cd713724f..fe00b4598b 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -35,8 +35,11 @@ config ARCH_DEFCONFIG
 config GCC_INDIRECT_THUNK
 	def_bool $(cc-option,-mindirect-branch-register)
 
+config CLANG_INDIRECT_THUNK
+	def_bool $(cc-option,-mretpoline-external-thunk)
+
 config INDIRECT_THUNK
-	depends on GCC_INDIRECT_THUNK
+	depends on GCC_INDIRECT_THUNK || CLANG_INDIRECT_THUNK
 	def_bool y
 
 config HAS_AS_CET_SS
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index 2da4bdb1ed..f2aa2a515f 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -47,6 +47,9 @@ CFLAGS-$(CONFIG_GCC_INDIRECT_THUNK) += -mindirect-branch=thunk-extern
 CFLAGS-$(CONFIG_GCC_INDIRECT_THUNK) += -mindirect-branch-register
 CFLAGS-$(CONFIG_GCC_INDIRECT_THUNK) += -fno-jump-tables
 
+# Enable clang retpoline support if available.
+CFLAGS-$(CONFIG_CLANG_INDIRECT_THUNK) += -mretpoline-external-thunk
+
 # If supported by the compiler, reduce stack alignment to 8 bytes. But allow
 # this to be overridden elsewhere.
 $(call cc-option-add,CFLAGS_stack_boundary,CC,-mpreferred-stack-boundary=3)
-- 
2.34.1

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

[Roger Pau Monne]

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>
---
 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 fe00b4598b..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
-	depends on GCC_INDIRECT_THUNK || CLANG_INDIRECT_THUNK
-	def_bool y
-
 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..3b0ba16a6a 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 calls are implemented using a new construct
+	  called "retpoline" that prevents speculation.
+
+	  If unsure, say Y.
+
 endmenu
 
 config HYPFS
-- 
2.34.1

Re: [PATCH 1/3] x86/retpoline: rename retpoline Kconfig check to include GCC prefix

[Jan Beulich]

On 16.02.2022 10:02, Roger Pau Monne wrote:
> Current retpoline checks apply to GCC only, so make it obvious by
> prefixing the Kconfig option with GCC. Keep the previous option as a
> way to signal generic retpoline support regardless of the underlying
> compiler.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

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

> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -32,9 +32,13 @@ config ARCH_DEFCONFIG
>  	string
>  	default "arch/x86/configs/x86_64_defconfig"
>  
> -config INDIRECT_THUNK
> +config GCC_INDIRECT_THUNK
>  	def_bool $(cc-option,-mindirect-branch-register)
>  
> +config INDIRECT_THUNK
> +	depends on GCC_INDIRECT_THUNK
> +	def_bool y

I think it is more logical (and more consistent) if "depends on"
would come after the basic type (i.e. the "def_bool" here).

Jan

Re: [PATCH 2/3] x86/clang: add retpoline support

[Jan Beulich]

On 16.02.2022 10:02, Roger Pau Monne wrote:
> Detect whether the compiler supports clang retpoline option and enable
> by default if available, just like it's done for gcc.
> 
> Note clang already disables jump tables when retpoline is enabled, so
> there's no need to also pass the fno-jump-tables parameter.

That's one of the secondary gcc side arrangements. What about the other
(-mindirect-branch-register)?

> Reported-by: Jan Beulich <JBeulich@suse.com>

That's Andrew, not me?

Jan

> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  xen/arch/x86/Kconfig | 5 ++++-
>  xen/arch/x86/arch.mk | 3 +++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 2cd713724f..fe00b4598b 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -35,8 +35,11 @@ config ARCH_DEFCONFIG
>  config GCC_INDIRECT_THUNK
>  	def_bool $(cc-option,-mindirect-branch-register)
>  
> +config CLANG_INDIRECT_THUNK
> +	def_bool $(cc-option,-mretpoline-external-thunk)
> +
>  config INDIRECT_THUNK
> -	depends on GCC_INDIRECT_THUNK
> +	depends on GCC_INDIRECT_THUNK || CLANG_INDIRECT_THUNK
>  	def_bool y
>  
>  config HAS_AS_CET_SS
> diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
> index 2da4bdb1ed..f2aa2a515f 100644
> --- a/xen/arch/x86/arch.mk
> +++ b/xen/arch/x86/arch.mk
> @@ -47,6 +47,9 @@ CFLAGS-$(CONFIG_GCC_INDIRECT_THUNK) += -mindirect-branch=thunk-extern
>  CFLAGS-$(CONFIG_GCC_INDIRECT_THUNK) += -mindirect-branch-register
>  CFLAGS-$(CONFIG_GCC_INDIRECT_THUNK) += -fno-jump-tables
>  
> +# Enable clang retpoline support if available.
> +CFLAGS-$(CONFIG_CLANG_INDIRECT_THUNK) += -mretpoline-external-thunk
> +
>  # If supported by the compiler, reduce stack alignment to 8 bytes. But allow
>  # this to be overridden elsewhere.
>  $(call cc-option-add,CFLAGS_stack_boundary,CC,-mpreferred-stack-boundary=3)

Re: [PATCH 2/3] x86/clang: add retpoline support

[Roger Pau Monné]

On Wed, Feb 16, 2022 at 10:47:52AM +0100, Jan Beulich wrote:
> On 16.02.2022 10:02, Roger Pau Monne wrote:
> > Detect whether the compiler supports clang retpoline option and enable
> > by default if available, just like it's done for gcc.
> > 
> > Note clang already disables jump tables when retpoline is enabled, so
> > there's no need to also pass the fno-jump-tables parameter.
> 
> That's one of the secondary gcc side arrangements. What about the other
> (-mindirect-branch-register)?

clang already passes the return address on a register always on amd64
so that's not needed either. I can add a comment like I've done for
no-jump-tables.

> > Reported-by: Jan Beulich <JBeulich@suse.com>
> 
> That's Andrew, not me?

Ops, sorry, I got that mixed up then.

Thanks, Roger.

Re: [PATCH 2/3] x86/clang: add retpoline support

[Jan Beulich]

On 16.02.2022 12:26, Roger Pau Monné wrote:
> On Wed, Feb 16, 2022 at 10:47:52AM +0100, Jan Beulich wrote:
>> On 16.02.2022 10:02, Roger Pau Monne wrote:
>>> Detect whether the compiler supports clang retpoline option and enable
>>> by default if available, just like it's done for gcc.
>>>
>>> Note clang already disables jump tables when retpoline is enabled, so
>>> there's no need to also pass the fno-jump-tables parameter.
>>
>> That's one of the secondary gcc side arrangements. What about the other
>> (-mindirect-branch-register)?
> 
> clang already passes the return address on a register always on amd64
> so that's not needed either. I can add a comment like I've done for
> no-jump-tables.
> 
>>> Reported-by: Jan Beulich <JBeulich@suse.com>
>>
>> That's Andrew, not me?
> 
> Ops, sorry, I got that mixed up then.

With the adjustments then
Reviewed-by: Jan Beulich <jbeulich@suse.com>