Add Kconfig, Makefile and basic instruction support for x86 IBT.
TODO: clang
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
arch/x86/Kconfig | 15 ++++++++++++
arch/x86/Makefile | 5 +++-
arch/x86/include/asm/ibt.h | 53 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 72 insertions(+), 1 deletion(-)
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1861,6 +1861,21 @@ config X86_UMIP
specific cases in protected and virtual-8086 modes. Emulated
results are dummy.
+config CC_HAS_IBT
+ # GCC >= 9 and binutils >= 2.29
+ # Retpoline check to work around https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93654
+ def_bool $(cc-option, -fcf-protection=branch -mindirect-branch-register) && $(as-instr,endbr64)
+
+config X86_IBT
+ prompt "Indirect Branch Tracking"
+ bool
+ depends on X86_64 && CC_HAS_IBT
+ help
+ Build the kernel with support for Indirect Branch Tracking, a
+ hardware supported CFI scheme. Any indirect call must land on
+ an ENDBR instruction, as such, the compiler will litter the
+ code with them to make this happen.
+
config X86_INTEL_MEMORY_PROTECTION_KEYS
prompt "Memory Protection Keys"
def_bool y
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -62,8 +62,11 @@ export BITS
#
KBUILD_CFLAGS += -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx
-# Intel CET isn't enabled in the kernel
+ifeq ($(CONFIG_X86_IBT),y)
+KBUILD_CFLAGS += $(call cc-option,-fcf-protection=branch)
+else
KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none)
+endif
ifeq ($(CONFIG_X86_32),y)
BITS := 32
--- /dev/null
+++ b/arch/x86/include/asm/ibt.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_IBT_H
+#define _ASM_X86_IBT_H
+
+#ifdef CONFIG_X86_IBT
+
+#ifndef __ASSEMBLY__
+
+#ifdef CONFIG_X86_64
+#define ASM_ENDBR "endbr64\n\t"
+#else
+#define ASM_ENDBR "endbr32\n\t"
+#endif
+
+#define __noendbr __attribute__((nocf_check))
+
+/*
+ * A bit convoluted, but matches both endbr32 and endbr64 without
+ * having either as literal in the text.
+ */
+static inline bool is_endbr(const void *addr)
+{
+ unsigned int val = ~*(unsigned int *)addr;
+ val |= 0x01000000U;
+ return val == ~0xfa1e0ff3;
+}
+
+#else /* __ASSEMBLY__ */
+
+#ifdef CONFIG_X86_64
+#define ENDBR endbr64
+#else
+#define ENDBR endbr32
+#endif
+
+#endif /* __ASSEMBLY__ */
+
+#else /* !IBT */
+
+#ifndef __ASSEMBLY__
+
+#define ASM_ENDBR
+
+#define __noendbr
+
+#else /* __ASSEMBLY__ */
+
+#define ENDBR
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* CONFIG_X86_IBT */
+#endif /* _ASM_X86_IBT_H */
On Fri, Feb 18, 2022 at 01:14:51PM -0800, Josh Poimboeuf wrote:
> On Fri, Feb 18, 2022 at 05:49:07PM +0100, Peter Zijlstra wrote:
> > +#ifdef CONFIG_X86_64
> > +#define ASM_ENDBR "endbr64\n\t"
> > +#else
> > +#define ASM_ENDBR "endbr32\n\t"
> > +#endif
>
> Is it safe to assume all supported assemblers know this instruction?
I was hoping the answer was yes, given CC_HAS_IBT.
From: Andrew Cooper
> Sent: 18 February 2022 20:50
>
> On 18/02/2022 16:49, Peter Zijlstra wrote:
> > +/*
> > + * A bit convoluted, but matches both endbr32 and endbr64 without
> > + * having either as literal in the text.
> > + */
> > +static inline bool is_endbr(const void *addr)
> > +{
> > + unsigned int val = ~*(unsigned int *)addr;
> > + val |= 0x01000000U;
> > + return val == ~0xfa1e0ff3;
> > +}
>
> At this point, I feel I've earned an "I told you so". :)
>
> Clang 13 sees straight through the trickery and generates:
>
> is_endbr: # @is_endbr
> movl $-16777217, %eax # imm = 0xFEFFFFFF
> andl (%rdi), %eax
> cmpl $-98693133, %eax # imm = 0xFA1E0FF3
> sete %al
> retq
I think it is enough to add:
asm("", "=r" (val));
somewhere in the middle.
(I think that is right for asm with input and output in the same
register.)
There might be a HIDE_FOR_OPTIMISER() define that does that.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
From: Andrew Cooper
> Sent: 18 February 2022 21:24
>
> On 18/02/2022 21:11, David Laight wrote:
> > From: Andrew Cooper
> >> Sent: 18 February 2022 20:50
> >>
> >> On 18/02/2022 16:49, Peter Zijlstra wrote:
> >>> +/*
> >>> + * A bit convoluted, but matches both endbr32 and endbr64 without
> >>> + * having either as literal in the text.
> >>> + */
> >>> +static inline bool is_endbr(const void *addr)
> >>> +{
> >>> + unsigned int val = ~*(unsigned int *)addr;
> >>> + val |= 0x01000000U;
> >>> + return val == ~0xfa1e0ff3;
> >>> +}
> >> At this point, I feel I've earned an "I told you so". :)
> >>
> >> Clang 13 sees straight through the trickery and generates:
> >>
> >> is_endbr: # @is_endbr
> >> movl $-16777217, %eax # imm = 0xFEFFFFFF
> >> andl (%rdi), %eax
> >> cmpl $-98693133, %eax # imm = 0xFA1E0FF3
> >> sete %al
> >> retq
> > I think it is enough to add:
> > asm("", "=r" (val));
> > somewhere in the middle.
>
> (First, you mean "+r" not "=r"),
I always double check....
> but no - the problem isn't val. It's
> `~0xfa1e0ff3` which the compiler is free to transform in several unsafe way.
Actually you could do (modulo stupid errors):
val = (*(unsigned int *)addr & ~0x01000000) ^ 0xff3;
asm("", "+r" (val));
return val ^ 0xfa1e0000;
which should be zero for endbra and non-zero overwise.
Shame the compiler will probably never use the flags from the final xor.
Converting to bool just adds code!
(I hate bool)
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Fri, Feb 18, 2022 at 05:49:07PM +0100, Peter Zijlstra wrote:
> +#ifdef CONFIG_X86_64
> +#define ASM_ENDBR "endbr64\n\t"
> +#else
> +#define ASM_ENDBR "endbr32\n\t"
> +#endif
Is it safe to assume all supported assemblers know this instruction?
--
Josh
> +config CC_HAS_IBT
> + # GCC >= 9 and binutils >= 2.29
> + # Retpoline check to work around
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93654
> + def_bool $(cc-option, -fcf-protection=branch
> -mindirect-branch-register) && $(as-instr,endbr64)
> +
Is -mindirect-branch-register breaks compiling with clang. Maybe we
should we do instead?
+ def_bool ($(cc-option, -fcf-protection=branch
-mindirect-branch-register) || $(cc-option, -mretpoline-external-thunk))
&& $(as-instr,endbr64)
On Fri, Feb 18, 2022 at 08:49:45PM +0000, Andrew Cooper wrote:
> On 18/02/2022 16:49, Peter Zijlstra wrote:
> > +/*
> > + * A bit convoluted, but matches both endbr32 and endbr64 without
> > + * having either as literal in the text.
> > + */
> > +static inline bool is_endbr(const void *addr)
> > +{
> > + unsigned int val = ~*(unsigned int *)addr;
> > + val |= 0x01000000U;
> > + return val == ~0xfa1e0ff3;
> > +}
>
> At this point, I feel I've earned an "I told you so". :)
Ha! I actually have a note to double-check this. But yes, I'll stuff
that piece of asm in so I can forget about it.
On Fri, 2022-02-18 at 17:49 +0100, Peter Zijlstra wrote:
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1861,6 +1861,21 @@ config X86_UMIP
> specific cases in protected and virtual-8086 modes.
> Emulated
> results are dummy.
>
> +config CC_HAS_IBT
> + # GCC >= 9 and binutils >= 2.29
> + # Retpoline check to work around
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93654
> + def_bool $(cc-option, -fcf-protection=branch -mindirect-
> branch-register) && $(as-instr,endbr64)
> +
> +config X86_IBT
> + prompt "Indirect Branch Tracking"
> + bool
> + depends on X86_64 && CC_HAS_IBT
> + help
> + Build the kernel with support for Indirect Branch Tracking,
> a
> + hardware supported CFI scheme. Any indirect call must land
> on
> + an ENDBR instruction, as such, the compiler will litter the
> + code with them to make this happen.
> +
>
Could you call this something more specific then just X86_IBT? Like
X86_KERNEL_IBT or something? It could get confusing if we add userspace
IBT, or if someone wants IBT for KVM guests without CFI in the kernel.