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

[PATCH v3 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: split retpoline compiler support into separate option
  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   | 14 ++++++++++++++
 3 files changed, 26 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH v3 1/3] x86/retpoline: split retpoline compiler support into separate option

[Roger Pau Monne]

Keep the previous option as a way to signal generic retpoline support
regardless of the underlying compiler, while introducing a new
CC_INDIRECT_THUNK that signals whether the underlying compiler
supports retpoline.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v2:
 - Make the compiler option check generic instead of GCC only.

Changes since v1:
 - Put def_bool before depend on.
---
 xen/arch/x86/Kconfig |  6 +++++-
 xen/arch/x86/arch.mk | 10 ++++++----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index b4abfca46f..a1a90da4fc 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 CC_INDIRECT_THUNK
 	def_bool $(cc-option,-mindirect-branch-register)
 
+config INDIRECT_THUNK
+	def_bool y
+	depends on CC_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 fa7cf38443..6388b1d93f 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -42,10 +42,12 @@ 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
+ifeq ($(CONFIG_INDIRECT_THUNK),y)
+# Compile with gcc thunk-extern, indirect-branch-register if available.
+CFLAGS-$(CONFIG_CC_IS_GCC) += -mindirect-branch=thunk-extern
+CFLAGS-$(CONFIG_CC_IS_GCC) += -mindirect-branch-register
+CFLAGS-$(CONFIG_CC_IS_GCC) += -fno-jump-tables
+endif
 
 # If supported by the compiler, reduce stack alignment to 8 bytes. But allow
 # this to be overridden elsewhere.
-- 
2.34.1

[PATCH v3 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. Also clang
already passes the return address on a register always on amd64, so
there's no need for any equivalent mindirect-branch-register
parameter.

Reported-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 | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index a1a90da4fc..f8dca4dc85 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -33,7 +33,9 @@ config ARCH_DEFCONFIG
 	default "arch/x86/configs/x86_64_defconfig"
 
 config CC_INDIRECT_THUNK
-	def_bool $(cc-option,-mindirect-branch-register)
+	def_bool $(cc-option,-mindirect-branch-register) || \
+	         $(cc-option,-mretpoline-external-thunk)
+
 
 config INDIRECT_THUNK
 	def_bool y
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index 6388b1d93f..066d749ba0 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -47,6 +47,9 @@ ifeq ($(CONFIG_INDIRECT_THUNK),y)
 CFLAGS-$(CONFIG_CC_IS_GCC) += -mindirect-branch=thunk-extern
 CFLAGS-$(CONFIG_CC_IS_GCC) += -mindirect-branch-register
 CFLAGS-$(CONFIG_CC_IS_GCC) += -fno-jump-tables
+
+# Enable clang retpoline support if available.
+CFLAGS-$(CONFIG_CC_IS_CLANG) += -mretpoline-external-thunk
 endif
 
 # If supported by the compiler, reduce stack alignment to 8 bytes. But allow
-- 
2.34.1

[PATCH v3 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>
---
Changes since v2:
 - Place first in the section.
 - Remove "If unsure".

Changes since v1:
 - Fix description of option to use indirect branches instead of
   indirect calls.
---
 xen/arch/x86/Kconfig |  5 -----
 xen/common/Kconfig   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index f8dca4dc85..eb4d1a949f 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -36,11 +36,6 @@ config CC_INDIRECT_THUNK
 	def_bool $(cc-option,-mindirect-branch-register) || \
 	         $(cc-option,-mretpoline-external-thunk)
 
-
-config INDIRECT_THUNK
-	def_bool y
-	depends on CC_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/common/Kconfig b/xen/common/Kconfig
index db687b1785..533b8f33e6 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -85,6 +85,20 @@ config STATIC_MEMORY
 
 menu "Speculative hardening"
 
+config INDIRECT_THUNK
+	bool "Speculative Branch Target Injection Protection"
+	depends on CC_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.
+
 config SPECULATIVE_HARDEN_ARRAY
 	bool "Speculative Array Hardening"
 	default y
-- 
2.34.1

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

[Andrew Cooper]

On 18/02/2022 14:34, Roger Pau Monne wrote:
> 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: split retpoline compiler support into separate option
>   x86/clang: add retpoline support
>   x86/Kconfig: introduce option to select retpoline usage

Thanks.  I think that's looking much better IMO.

One thing, we want CC_HAS_* for consistency.  I can fix this on commit.

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

[Roger Pau Monné]

On Fri, Feb 18, 2022 at 03:45:12PM +0000, Andrew Cooper wrote:
> On 18/02/2022 14:34, Roger Pau Monne wrote:
> > 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: split retpoline compiler support into separate option
> >   x86/clang: add retpoline support
> >   x86/Kconfig: introduce option to select retpoline usage
> 
> Thanks.  I think that's looking much better IMO.
> 
> One thing, we want CC_HAS_* for consistency.  I can fix this on commit.
> 
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Could the patches be backported to stable-4.16?

Thanks, Roger.

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

[Jan Beulich]

On 30.03.2022 12:21, Roger Pau Monné wrote:
> On Fri, Feb 18, 2022 at 03:45:12PM +0000, Andrew Cooper wrote:
>> On 18/02/2022 14:34, Roger Pau Monne wrote:
>>> 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: split retpoline compiler support into separate option
>>>   x86/clang: add retpoline support
>>>   x86/Kconfig: introduce option to select retpoline usage
>>
>> Thanks.  I think that's looking much better IMO.
>>
>> One thing, we want CC_HAS_* for consistency.  I can fix this on commit.
>>
>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Could the patches be backported to stable-4.16?

Hmm, generally I'd view them as a feature rather than a bug fix, but
since you're explicitly requesting their backporting I guess I should
rather look at them from an improved-security angle.

Jan