[PATCH v23 0/9] Control-flow Enforcement: Indirect Branch Tracking

Control-flow Enforcement (CET) is a new Intel processor feature that blocks
return/jump-oriented programming attacks.  Details are in "Intel 64 and
IA-32 Architectures Software Developer's Manual" [1].

This is the second part of CET and enables Indirect Branch Tracking (IBT).
It is built on top of the shadow stack series.

Changes in v23:
- Add patch #6: introduce a macro for ENDBR instructions.
- Patch #7: replace endbr32 with ENDBR macro.
- Patch #9: revise, add/replace endbr64 with ENDBR macro.
- Rebase to Linus tree v5.12-rc3.

[1] Intel 64 and IA-32 Architectures Software Developer's Manual:

    https://software.intel.com/en-us/download/intel-64-and-ia-32-
    architectures-sdm-combined-volumes-1-2a-2b-2c-2d-3a-3b-3c-3d-and-4

[2] Indirect Branch Tracking patches v22:

    https://lore.kernel.org/r/20210310220519.16811-1-yu-cheng.yu@intel.com/

H.J. Lu (3):
  x86/cet/ibt: Update arch_prctl functions for Indirect Branch Tracking
  x86/vdso/32: Add ENDBR to __kernel_vsyscall entry point
  x86/vdso: Insert endbr32/endbr64 to vDSO

Yu-cheng Yu (6):
  x86/cet/ibt: Update Kconfig for user-mode Indirect Branch Tracking
  x86/cet/ibt: User-mode Indirect Branch Tracking support
  x86/cet/ibt: Handle signals for Indirect Branch Tracking
  x86/cet/ibt: Update ELF header parsing for Indirect Branch Tracking
  x86/entry: Introduce ENDBR macro
  x86/vdso: Add ENDBR to __vdso_sgx_enter_enclave

 arch/x86/Kconfig                         |  1 +
 arch/x86/entry/calling.h                 | 18 ++++++++
 arch/x86/entry/vdso/Makefile             |  4 ++
 arch/x86/entry/vdso/vdso32/system_call.S |  2 +
 arch/x86/entry/vdso/vsgx.S               |  4 ++
 arch/x86/include/asm/cet.h               |  3 ++
 arch/x86/kernel/cet.c                    | 59 +++++++++++++++++++++++-
 arch/x86/kernel/cet_prctl.c              |  5 ++
 arch/x86/kernel/fpu/signal.c             |  8 ++--
 arch/x86/kernel/process_64.c             |  8 ++++
 10 files changed, 107 insertions(+), 5 deletions(-)

-- 
2.21.0

Indirect branch tracking is a hardware security feature that verifies near
indirect call/jump instructions arrive at intended targets, which are
labeled by the compiler with ENDBR opcodes.  If such instructions reach
unlabeled locations, the processor raises control-protection faults.

Check the compiler is up-to-date at config time.

Signed-off-by: Yu-cheng Yu <redacted>
Reviewed-by: Kees Cook <redacted>
---
 arch/x86/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2c93178262f5..96000ed48469 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig

@@ -1953,6 +1953,7 @@ config X86_CET
 	def_bool n
 	depends on AS_WRUSS
 	depends on ARCH_HAS_SHADOW_STACK
+	depends on $(cc-option,-fcf-protection)
 	select ARCH_USES_HIGH_VMA_FLAGS
 	select ARCH_MAYBE_MKWRITE
 	select ARCH_USE_GNU_PROPERTY

-- 
2.21.0

An ELF file's .note.gnu.property indicates features the file supports.
The property is parsed at loading time and passed to arch_setup_elf_
property().  Update it for Indirect Branch Tracking.

Signed-off-by: Yu-cheng Yu <redacted>
Reviewed-by: Kees Cook <redacted>
---
 arch/x86/kernel/process_64.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index cda830b0f7ee..11497689a841 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c

@@ -864,6 +864,14 @@ int arch_setup_elf_property(struct arch_elf_state *state)
 			r = cet_setup_shstk();
 	}
 
+	if (r < 0)
+		return r;
+
+	if (static_cpu_has(X86_FEATURE_IBT)) {
+		if (state->gnu_property & GNU_PROPERTY_X86_FEATURE_1_IBT)
+			r = cet_setup_ibt();
+	}
+
 	return r;
 }
 #endif

-- 
2.21.0

When an indirect CALL/JMP instruction is executed and before it reaches
the target, it is in 'WAIT_ENDBR' status, which can be read from
MSR_IA32_U_CET.  The status is part of a task's status before a signal is
raised and preserved in the signal frame.  It is restored for sigreturn.

IBT state machine is described in Intel SDM Vol. 1, Sec. 18.3.

Signed-off-by: Yu-cheng Yu <redacted>
Reviewed-by: Kees Cook <redacted>
---
 arch/x86/kernel/cet.c        | 26 ++++++++++++++++++++++++--
 arch/x86/kernel/fpu/signal.c |  8 +++++---
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
index 3361706ba950..34a26eb7f259 100644
--- a/arch/x86/kernel/cet.c
+++ b/arch/x86/kernel/cet.c

@@ -300,6 +300,13 @@ void cet_restore_signal(struct sc_ext *sc_ext)
 		msr_val |= CET_SHSTK_EN;
 	}
 
+	if (cet->ibt_enabled) {
+		msr_val |= (CET_ENDBR_EN | CET_NO_TRACK_EN);
+
+		if (sc_ext->wait_endbr)
+			msr_val |= CET_WAIT_ENDBR;
+	}
+
 	if (test_thread_flag(TIF_NEED_FPU_LOAD))
 		cet_user_state->user_cet = msr_val;
 	else

@@ -340,9 +347,24 @@ int cet_setup_signal(bool ia32, unsigned long rstor_addr, struct sc_ext *sc_ext)
 		sc_ext->ssp = new_ssp;
 	}
 
-	if (ssp) {
+	if (ssp || cet->ibt_enabled) {
 		start_update_msrs();
-		wrmsrl(MSR_IA32_PL3_SSP, ssp);
+
+		if (ssp)
+			wrmsrl(MSR_IA32_PL3_SSP, ssp);
+
+		if (cet->ibt_enabled) {
+			u64 r;
+
+			rdmsrl(MSR_IA32_U_CET, r);
+
+			if (r & CET_WAIT_ENDBR) {
+				sc_ext->wait_endbr = 1;
+				r &= ~CET_WAIT_ENDBR;
+				wrmsrl(MSR_IA32_U_CET, r);
+			}
+		}
+
 		end_update_msrs();
 	}
 

diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 270e4649f435..b914d74c8ba6 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c

@@ -57,7 +57,8 @@ int save_cet_to_sigframe(int ia32, void __user *fp, unsigned long restorer)
 {
 	int err = 0;
 
-	if (!current->thread.cet.shstk_size)
+	if (!current->thread.cet.shstk_size &&
+	    !current->thread.cet.ibt_enabled)
 		return 0;
 
 	if (fp) {

@@ -89,7 +90,8 @@ static int get_cet_from_sigframe(int ia32, void __user *fp, struct sc_ext *ext)
 
 	memset(ext, 0, sizeof(*ext));
 
-	if (!current->thread.cet.shstk_size)
+	if (!current->thread.cet.shstk_size &&
+	    !current->thread.cet.ibt_enabled)
 		return 0;
 
 	if (fp) {

@@ -577,7 +579,7 @@ static unsigned long fpu__alloc_sigcontext_ext(unsigned long sp)
 	 * sigcontext_ext is at: fpu + fpu_user_xstate_size +
 	 * FP_XSTATE_MAGIC2_SIZE, then aligned to 8.
 	 */
-	if (cet->shstk_size)
+	if (cet->shstk_size || cet->ibt_enabled)
 		sp -= (sizeof(struct sc_ext) + 8);
 
 	return sp;

-- 
2.21.0

From: "H.J. Lu" <redacted>

Update ARCH_X86_CET_STATUS and ARCH_X86_CET_DISABLE for Indirect Branch
Tracking.

Signed-off-by: H.J. Lu <redacted>
Signed-off-by: Yu-cheng Yu <redacted>
Reviewed-by: Kees Cook <redacted>
---
 arch/x86/kernel/cet_prctl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kernel/cet_prctl.c b/arch/x86/kernel/cet_prctl.c
index 0030c63a08c0..4df1eac41965 100644
--- a/arch/x86/kernel/cet_prctl.c
+++ b/arch/x86/kernel/cet_prctl.c

@@ -22,6 +22,9 @@ static int cet_copy_status_to_user(struct cet_status *cet, u64 __user *ubuf)
 		buf[2] = cet->shstk_size;
 	}
 
+	if (cet->ibt_enabled)
+		buf[0] |= GNU_PROPERTY_X86_FEATURE_1_IBT;
+
 	return copy_to_user(ubuf, buf, sizeof(buf));
 }
 

@@ -46,6 +49,8 @@ int prctl_cet(int option, u64 arg2)
 			return -EINVAL;
 		if (arg2 & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
 			cet_disable_shstk();
+		if (arg2 & GNU_PROPERTY_X86_FEATURE_1_IBT)
+			cet_disable_ibt();
 		return 0;
 
 	case ARCH_X86_CET_LOCK:

-- 
2.21.0

ENDBR is a special new instruction for the Indirect Branch Tracking (IBT)
component of CET.  IBT prevents attacks by ensuring that (most) indirect
branches and function calls may only land at ENDBR instructions.  Branches
that don't follow the rules will result in control flow (#CF) exceptions.

ENDBR is a noop when IBT is unsupported or disabled.  Most ENDBR
instructions are inserted automatically by the compiler, but branch
targets written in assembly must have ENDBR added manually.

There are two ENDBR versions: one for 64-bit and the other for 32.
Introduce a macro to eliminate ifdeffery at call sites.

Signed-off-by: Yu-cheng Yu <redacted>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/x86/entry/calling.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index 07a9331d55e7..a63d33f7f069 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h

@@ -392,3 +392,21 @@ For 32-bit we have the following conventions - kernel is built with
 .endm
 
 #endif /* CONFIG_SMP */
+/*
+ * ENDBR is an instruction for the Indirect Branch Tracking (IBT) component
+ * of CET.  IBT prevents attacks by ensuring that (most) indirect branches
+ * function calls may only land at ENDBR instructions.  Branches that don't
+ * follow the rules will result in control flow (#CF) exceptions.
+ * ENDBR is a noop when IBT is unsupported or disabled.  Most ENDBR
+ * instructions are inserted automatically by the compiler, but branch
+ * targets written in assembly must have ENDBR added manually.
+ */
+.macro ENDBR
+#ifdef CONFIG_X86_CET
+#ifdef __i386__
+	endbr32
+#else
+	endbr64
+#endif
+#endif
+.endm

-- 
2.21.0

On 3/16/21 8:13 AM, Yu-cheng Yu wrote:

Is "#ifdef __i386__" the right thing to use here?  I guess ENDBR only
ends up getting used in the VDSO, but there's a lot of
non-userspace-exposed stuff in calling.h.  It seems a bit weird to have
the normally userspace-only __i386__ in there.

I don't see any existing direct use of __i386__ in arch/x86/entry/vdso.

On 3/16/2021 8:49 AM, Dave Hansen wrote:

On 3/16/21 8:13 AM, Yu-cheng Yu wrote:

quoted

--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h

@@ -392,3 +392,21 @@ For 32-bit we have the following conventions - kernel is built with
  .endm
  
  #endif /* CONFIG_SMP */
+/*
+ * ENDBR is an instruction for the Indirect Branch Tracking (IBT) component
+ * of CET.  IBT prevents attacks by ensuring that (most) indirect branches
+ * function calls may only land at ENDBR instructions.  Branches that don't
+ * follow the rules will result in control flow (#CF) exceptions.
+ * ENDBR is a noop when IBT is unsupported or disabled.  Most ENDBR
+ * instructions are inserted automatically by the compiler, but branch
+ * targets written in assembly must have ENDBR added manually.
+ */
+.macro ENDBR
+#ifdef CONFIG_X86_CET
+#ifdef __i386__
+	endbr32
+#else
+	endbr64
+#endif
+#endif
+.endm

Is "#ifdef __i386__" the right thing to use here?  I guess ENDBR only
ends up getting used in the VDSO, but there's a lot of
non-userspace-exposed stuff in calling.h.  It seems a bit weird to have
the normally userspace-only __i386__ in there.

I don't see any existing direct use of __i386__ in arch/x86/entry/vdso.

Good point.  My thought was, __i386__ comes from the compiler having the 
-m32 command-line option, and it is not dependent on anything else.

Alternatively, there is another compiler-defined macro _CET_ENDBR that 
can be used.  We can put the following in calling.h:

#ifdef __CET__
#include <cet.h>
#else
#define _CET_ENDBR
#endif

and then use _CET_ENDBR in other files.  How is that?

In the future, in case we have kernel-mode IBT, ENDBR macros are also 
needed for other assembly files.

Thanks,
Yu-cheng

On 3/16/21 10:12 AM, Yu, Yu-cheng wrote:

On 3/16/2021 8:49 AM, Dave Hansen wrote:

...

quoted

Is "#ifdef __i386__" the right thing to use here?  I guess ENDBR only
ends up getting used in the VDSO, but there's a lot of
non-userspace-exposed stuff in calling.h.  It seems a bit weird to have
the normally userspace-only __i386__ in there.

I don't see any existing direct use of __i386__ in arch/x86/entry/vdso.

Good point.  My thought was, __i386__ comes from the compiler having the
-m32 command-line option, and it is not dependent on anything else.

Alternatively, there is another compiler-defined macro _CET_ENDBR that
can be used.  We can put the following in calling.h:

#ifdef __CET__
#include <cet.h>
#else
#define _CET_ENDBR
#endif

and then use _CET_ENDBR in other files.  How is that?

In the future, in case we have kernel-mode IBT, ENDBR macros are also
needed for other assembly files.

First of all, I think putting the macro in calling.h is wrong if it will
be used exclusively in the VDSO.  If it's VDSO-only, please put it in a
local header in the vdso/ directory, maybe even a new cet.h.

Also, Boris asked for two *different* macros for 32 and 64-bit:

https://lore.kernel.org/linux-api/20210310231731.GK23521@zn.tnic/

Could you do that in the next version, please?

On 3/16/2021 10:28 AM, Dave Hansen wrote:

On 3/16/21 10:12 AM, Yu, Yu-cheng wrote:

quoted

On 3/16/2021 8:49 AM, Dave Hansen wrote:

...

quoted

quoted

Is "#ifdef __i386__" the right thing to use here?  I guess ENDBR only
ends up getting used in the VDSO, but there's a lot of
non-userspace-exposed stuff in calling.h.  It seems a bit weird to have
the normally userspace-only __i386__ in there.

I don't see any existing direct use of __i386__ in arch/x86/entry/vdso.

Good point.  My thought was, __i386__ comes from the compiler having the
-m32 command-line option, and it is not dependent on anything else.

Alternatively, there is another compiler-defined macro _CET_ENDBR that
can be used.  We can put the following in calling.h:

#ifdef __CET__
#include <cet.h>
#else
#define _CET_ENDBR
#endif

and then use _CET_ENDBR in other files.  How is that?

In the future, in case we have kernel-mode IBT, ENDBR macros are also
needed for other assembly files.

First of all, I think putting the macro in calling.h is wrong if it will
be used exclusively in the VDSO.  If it's VDSO-only, please put it in a
local header in the vdso/ directory, maybe even a new cet.h.

Also, Boris asked for two *different* macros for 32 and 64-bit:

https://lore.kernel.org/linux-api/20210310231731.GK23521@zn.tnic/

Could you do that in the next version, please?

Yes, we can do two macros, probably in arch/x86/include/asm/vdso.h.

On 3/16/21 10:44 AM, Yu, Yu-cheng wrote:

quoted

Also, Boris asked for two *different* macros for 32 and 64-bit:

https://lore.kernel.org/linux-api/20210310231731.GK23521@zn.tnic/

Could you do that in the next version, please?

Yes, we can do two macros, probably in arch/x86/include/asm/vdso.h.

*But*, please do leverage _CET_ENDBR if you can.  It seems awfully close
to what we need.  If it works out, just use it for the 32/64-bit switch.

On Tue, Mar 16, 2021 at 10:12:39AM -0700, Yu, Yu-cheng wrote:

Alternatively, there is another compiler-defined macro _CET_ENDBR that can
be used.  We can put the following in calling.h:

Not calling.h - this is apparently needed in vdso code only so I guess
some header there, arch/x86/include/asm/vdso.h maybe? In the

#else /* __ASSEMBLER__ */

branch maybe...

#ifdef __CET__
#include <cet.h>
#else
#define _CET_ENDBR
#endif

and then use _CET_ENDBR in other files.  How is that?

What does that macro do? Issue an ENDBR only?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

On 3/16/2021 10:30 AM, Borislav Petkov wrote:

On Tue, Mar 16, 2021 at 10:12:39AM -0700, Yu, Yu-cheng wrote:

quoted

Alternatively, there is another compiler-defined macro _CET_ENDBR that can
be used.  We can put the following in calling.h:

Not calling.h - this is apparently needed in vdso code only so I guess
some header there, arch/x86/include/asm/vdso.h maybe? In the

#else /* __ASSEMBLER__ */

branch maybe...

quoted

#ifdef __CET__
#include <cet.h>
#else
#define _CET_ENDBR
#endif

and then use _CET_ENDBR in other files.  How is that?

What does that macro do? Issue an ENDBR only?

Yes, issue endbr32, endbr64, or nothing when cet is not enabled.

On Tue, Mar 16, 2021 at 10:12:39AM -0700, Yu, Yu-cheng wrote:

Alternatively, there is another compiler-defined macro _CET_ENDBR that can
be used.  We can put the following in calling.h:

#ifdef __CET__
#include <cet.h>
#else
#define _CET_ENDBR
#endif

and then use _CET_ENDBR in other files.  How is that?

In the future, in case we have kernel-mode IBT, ENDBR macros are also needed
for other assembly files.

Can we please call it IBT_ENDBR or just ENDBR. CET is a horrible name,
seeing how it is not specific.

On 3/16/2021 12:57 PM, Peter Zijlstra wrote:

On Tue, Mar 16, 2021 at 10:12:39AM -0700, Yu, Yu-cheng wrote:

quoted

Alternatively, there is another compiler-defined macro _CET_ENDBR that can
be used.  We can put the following in calling.h:

#ifdef __CET__
#include <cet.h>
#else
#define _CET_ENDBR
#endif

and then use _CET_ENDBR in other files.  How is that?

In the future, in case we have kernel-mode IBT, ENDBR macros are also needed
for other assembly files.

Can we please call it IBT_ENDBR or just ENDBR. CET is a horrible name,
seeing how it is not specific.

_CET_ENDBR is from the compiler and we cannot change it.  We can do:

#define ENDBR _CET_ENDBR

How is that?

On Tue, Mar 16, 2021 at 01:05:30PM -0700, Yu, Yu-cheng wrote:

On 3/16/2021 12:57 PM, Peter Zijlstra wrote:

quoted

On Tue, Mar 16, 2021 at 10:12:39AM -0700, Yu, Yu-cheng wrote:

quoted

Alternatively, there is another compiler-defined macro _CET_ENDBR that can
be used.  We can put the following in calling.h:

#ifdef __CET__
#include <cet.h>
#else
#define _CET_ENDBR
#endif

and then use _CET_ENDBR in other files.  How is that?

In the future, in case we have kernel-mode IBT, ENDBR macros are also needed
for other assembly files.

Can we please call it IBT_ENDBR or just ENDBR. CET is a horrible name,
seeing how it is not specific.

_CET_ENDBR is from the compiler and we cannot change it.  We can do:

#define ENDBR _CET_ENDBR

How is that?

Do we really want to include compiler headers? AFAICT it's not a
built-in. Also what about clang?

This thing seems trivial enough to build our own, it's a single damn
instruction. That also means we don't have to worry about changes to
header files we don't control.

On 3/16/2021 1:15 PM, Peter Zijlstra wrote:

On Tue, Mar 16, 2021 at 01:05:30PM -0700, Yu, Yu-cheng wrote:

quoted

On 3/16/2021 12:57 PM, Peter Zijlstra wrote:

quoted

On Tue, Mar 16, 2021 at 10:12:39AM -0700, Yu, Yu-cheng wrote:

quoted

Alternatively, there is another compiler-defined macro _CET_ENDBR that can
be used.  We can put the following in calling.h:

#ifdef __CET__
#include <cet.h>
#else
#define _CET_ENDBR
#endif

and then use _CET_ENDBR in other files.  How is that?

In the future, in case we have kernel-mode IBT, ENDBR macros are also needed
for other assembly files.

Can we please call it IBT_ENDBR or just ENDBR. CET is a horrible name,
seeing how it is not specific.

_CET_ENDBR is from the compiler and we cannot change it.  We can do:

#define ENDBR _CET_ENDBR

How is that?

Do we really want to include compiler headers? AFAICT it's not a
built-in. Also what about clang?

This thing seems trivial enough to build our own, it's a single damn
instruction. That also means we don't have to worry about changes to
header files we don't control.

Then, what about moving what I had earlier to vdso.h?
If we don't want __i386__ either, then make it two macros.

+.macro ENDBR
+#ifdef CONFIG_X86_CET
+#ifdef __i386__
+	endbr32
+#else
+	endbr64
+#endif
+#endif
+.endm

On 3/16/2021 1:26 PM, Yu, Yu-cheng wrote:

On 3/16/2021 1:15 PM, Peter Zijlstra wrote:

quoted

On Tue, Mar 16, 2021 at 01:05:30PM -0700, Yu, Yu-cheng wrote:

quoted

On 3/16/2021 12:57 PM, Peter Zijlstra wrote:

quoted

On Tue, Mar 16, 2021 at 10:12:39AM -0700, Yu, Yu-cheng wrote:

quoted

Alternatively, there is another compiler-defined macro _CET_ENDBR 
that can
be used.  We can put the following in calling.h:

#ifdef __CET__
#include <cet.h>
#else
#define _CET_ENDBR
#endif

and then use _CET_ENDBR in other files.  How is that?

In the future, in case we have kernel-mode IBT, ENDBR macros are 
also needed
for other assembly files.

Can we please call it IBT_ENDBR or just ENDBR. CET is a horrible name,
seeing how it is not specific.

_CET_ENDBR is from the compiler and we cannot change it.  We can do:

#define ENDBR _CET_ENDBR

How is that?

Do we really want to include compiler headers? AFAICT it's not a
built-in. Also what about clang?

This thing seems trivial enough to build our own, it's a single damn
instruction. That also means we don't have to worry about changes to
header files we don't control.

Then, what about moving what I had earlier to vdso.h?
If we don't want __i386__ either, then make it two macros.

+.macro ENDBR
+#ifdef CONFIG_X86_CET
+#ifdef __i386__
+    endbr32
+#else
+    endbr64
+#endif
+#endif
+.endm

I will make it like the following:

diff --git a/arch/x86/include/asm/vdso.h b/arch/x86/include/asm/vdso.h
index 98aa103eb4ab..4c0262dcb93d 100644
--- a/arch/x86/include/asm/vdso.h
+++ b/arch/x86/include/asm/vdso.h

@@ -52,6 +52,15 @@ extern int map_vdso_once(const struct vdso_image 

*image, unsigned long addr);
  extern bool fixup_vdso_exception(struct pt_regs *regs, int trapnr,
  				 unsigned long error_code,
  				 unsigned long fault_addr);
-#endif /* __ASSEMBLER__ */
+#else /* __ASSEMBLER__ */
+
+#ifdef CONFIG_X86_CET
+#define ENDBR64 endbr64
+#define ENDBR32 endbr32
+#else /*!CONFIG_X86_CET */
+#define ENDBR64
+#define ENDBR32
+#endif

+#endif /* __ASSEMBLER__ */
  #endif /* _ASM_X86_VDSO_H */

On Tue, Mar 16, 2021 at 01:26:52PM -0700, Yu, Yu-cheng wrote:

Then, what about moving what I had earlier to vdso.h?
If we don't want __i386__ either, then make it two macros.

vdso.h seems to use CONFIG_X86_{64,32} resp.

+.macro ENDBR
+#ifdef CONFIG_X86_CET

And shouldn't that be CONFIG_X86_IBT ?

+#ifdef __i386__

#ifdef CONFIG_X86_32

+	endbr32
+#else
+	endbr64
+#endif
+#endif
+.endm

Introduce user-mode Indirect Branch Tracking (IBT) support.  Add routines
for the setup/disable of IBT.

Signed-off-by: Yu-cheng Yu <redacted>
Reviewed-by: Kees Cook <redacted>
---
 arch/x86/include/asm/cet.h |  3 +++
 arch/x86/kernel/cet.c      | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/arch/x86/include/asm/cet.h b/arch/x86/include/asm/cet.h
index c2437378f339..c20c2f671145 100644
--- a/arch/x86/include/asm/cet.h
+++ b/arch/x86/include/asm/cet.h

@@ -15,6 +15,7 @@ struct cet_status {
 	unsigned long	shstk_base;
 	unsigned long	shstk_size;
 	unsigned int	locked:1;
+	unsigned int	ibt_enabled:1;
 };
 
 #ifdef CONFIG_X86_CET

@@ -27,6 +28,8 @@ void cet_free_shstk(struct task_struct *p);
 int cet_verify_rstor_token(bool ia32, unsigned long ssp, unsigned long *new_ssp);
 void cet_restore_signal(struct sc_ext *sc);
 int cet_setup_signal(bool ia32, unsigned long rstor, struct sc_ext *sc);
+int cet_setup_ibt(void);
+void cet_disable_ibt(void);
 #else
 static inline int prctl_cet(int option, u64 arg2) { return -EINVAL; }
 static inline int cet_setup_thread_shstk(struct task_struct *p,

diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
index 12738cdfb5f2..3361706ba950 100644
--- a/arch/x86/kernel/cet.c
+++ b/arch/x86/kernel/cet.c

@@ -13,6 +13,8 @@
 #include <linux/uaccess.h>
 #include <linux/sched/signal.h>
 #include <linux/compat.h>
+#include <linux/vmalloc.h>
+#include <linux/bitops.h>
 #include <asm/msr.h>
 #include <asm/user.h>
 #include <asm/fpu/internal.h>

@@ -346,3 +348,34 @@ int cet_setup_signal(bool ia32, unsigned long rstor_addr, struct sc_ext *sc_ext)
 
 	return 0;
 }
+
+int cet_setup_ibt(void)
+{
+	u64 msr_val;
+
+	if (!static_cpu_has(X86_FEATURE_IBT))
+		return -EOPNOTSUPP;
+
+	start_update_msrs();
+	rdmsrl(MSR_IA32_U_CET, msr_val);
+	msr_val |= (CET_ENDBR_EN | CET_NO_TRACK_EN);
+	wrmsrl(MSR_IA32_U_CET, msr_val);
+	end_update_msrs();
+	current->thread.cet.ibt_enabled = 1;
+	return 0;
+}
+
+void cet_disable_ibt(void)
+{
+	u64 msr_val;
+
+	if (!static_cpu_has(X86_FEATURE_IBT))
+		return;
+
+	start_update_msrs();
+	rdmsrl(MSR_IA32_U_CET, msr_val);
+	msr_val &= ~CET_ENDBR_EN;
+	wrmsrl(MSR_IA32_U_CET, msr_val);
+	end_update_msrs();
+	current->thread.cet.ibt_enabled = 0;
+}

-- 
2.21.0

From: "H.J. Lu" <redacted>

ENDBR is a special new instruction for the Indirect Branch Tracking (IBT)
component of CET.  IBT prevents attacks by ensuring that (most) indirect
branches and function calls may only land at ENDBR instructions.  Branches
that don't follow the rules will result in control flow (#CF) exceptions.

ENDBR is a noop when IBT is unsupported or disabled.  Most ENDBR
instructions are inserted automatically by the compiler, but branch
targets written in assembly must have ENDBR added manually.

Add that to __kernel_vsyscall entry point.

Signed-off-by: H.J. Lu <redacted>
Signed-off-by: Yu-cheng Yu <redacted>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <redacted>
---
 arch/x86/entry/vdso/vdso32/system_call.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/entry/vdso/vdso32/system_call.S b/arch/x86/entry/vdso/vdso32/system_call.S
index de1fff7188aa..adbe948c1a81 100644
--- a/arch/x86/entry/vdso/vdso32/system_call.S
+++ b/arch/x86/entry/vdso/vdso32/system_call.S

@@ -7,6 +7,7 @@
 #include <asm/dwarf2.h>
 #include <asm/cpufeatures.h>
 #include <asm/alternative-asm.h>
+#include "../../calling.h"
 
 	.text
 	.globl __kernel_vsyscall

@@ -14,6 +15,7 @@
 	ALIGN
 __kernel_vsyscall:
 	CFI_STARTPROC
+	ENDBR
 	/*
 	 * Reshuffle regs so that all of any of the entry instructions
 	 * will preserve enough state.

-- 
2.21.0

From: "H.J. Lu" <redacted>

When Indirect Branch Tracking (IBT) is enabled, vDSO functions may be
called indirectly, and must have ENDBR32 or ENDBR64 as the first
instruction.  The compiler must support -fcf-protection=branch so that it
can be used to compile vDSO.

Signed-off-by: H.J. Lu <redacted>
Signed-off-by: Yu-cheng Yu <redacted>
Acked-by: Andy Lutomirski <luto@kernel.org>
Reviewed-by: Kees Cook <redacted>
---
 arch/x86/entry/vdso/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 05c4abc2fdfd..c9eccbc06e8c 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile

@@ -93,6 +93,10 @@ endif
 
 $(vobjs): KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_LTO) $(GCC_PLUGINS_CFLAGS) $(RETPOLINE_CFLAGS),$(KBUILD_CFLAGS)) $(CFL)
 
+ifdef CONFIG_X86_CET
+$(vobjs) $(vobjs32): KBUILD_CFLAGS += -fcf-protection=branch
+endif
+
 #
 # vDSO code runs in userspace and -pg doesn't help with profiling anyway.
 #

-- 
2.21.0

ENDBR is a special new instruction for the Indirect Branch Tracking (IBT)
component of CET.  IBT prevents attacks by ensuring that (most) indirect
branches and function calls may only land at ENDBR instructions.  Branches
that don't follow the rules will result in control flow (#CF) exceptions.

ENDBR is a noop when IBT is unsupported or disabled.  Most ENDBR
instructions are inserted automatically by the compiler, but branch
targets written in assembly must have ENDBR added manually.

Add ENDBR to __vdso_sgx_enter_enclave() branch targets.

Signed-off-by: Yu-cheng Yu <redacted>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/x86/entry/vdso/vsgx.S | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/entry/vdso/vsgx.S b/arch/x86/entry/vdso/vsgx.S
index 86a0e94f68df..1baa9b49053e 100644
--- a/arch/x86/entry/vdso/vsgx.S
+++ b/arch/x86/entry/vdso/vsgx.S

@@ -6,6 +6,7 @@
 #include <asm/enclu.h>
 
 #include "extable.h"
+#include "../calling.h"
 
 /* Relative to %rbp. */
 #define SGX_ENCLAVE_OFFSET_OF_RUN		16

@@ -27,6 +28,7 @@
 SYM_FUNC_START(__vdso_sgx_enter_enclave)
 	/* Prolog */
 	.cfi_startproc
+	ENDBR
 	push	%rbp
 	.cfi_adjust_cfa_offset	8
 	.cfi_rel_offset		%rbp, 0

@@ -62,6 +64,7 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
 .Lasync_exit_pointer:
 .Lenclu_eenter_eresume:
 	enclu
+	ENDBR
 
 	/* EEXIT jumps here unless the enclave is doing something fancy. */
 	mov	SGX_ENCLAVE_OFFSET_OF_RUN(%rbp), %rbx

@@ -91,6 +94,7 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
 	jmp	.Lout
 
 .Lhandle_exception:
+	ENDBR
 	mov	SGX_ENCLAVE_OFFSET_OF_RUN(%rbp), %rbx
 
 	/* Set the exception info. */

-- 
2.21.0

On Tue, Mar 16, 2021 at 08:13:19AM -0700, Yu-cheng Yu wrote:

Looks good to me.

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

/Jarkko

On 3/16/2021 12:22 PM, Jarkko Sakkinen wrote:

On Tue, Mar 16, 2021 at 08:13:19AM -0700, Yu-cheng Yu wrote:

quoted

ENDBR is a special new instruction for the Indirect Branch Tracking (IBT)
component of CET.  IBT prevents attacks by ensuring that (most) indirect
branches and function calls may only land at ENDBR instructions.  Branches
that don't follow the rules will result in control flow (#CF) exceptions.

ENDBR is a noop when IBT is unsupported or disabled.  Most ENDBR
instructions are inserted automatically by the compiler, but branch
targets written in assembly must have ENDBR added manually.

Add ENDBR to __vdso_sgx_enter_enclave() branch targets.

Signed-off-by: Yu-cheng Yu <redacted>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
  arch/x86/entry/vdso/vsgx.S | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/arch/x86/entry/vdso/vsgx.S b/arch/x86/entry/vdso/vsgx.S
index 86a0e94f68df..1baa9b49053e 100644
--- a/arch/x86/entry/vdso/vsgx.S
+++ b/arch/x86/entry/vdso/vsgx.S

@@ -6,6 +6,7 @@
  #include <asm/enclu.h>
  
  #include "extable.h"
+#include "../calling.h"
  
  /* Relative to %rbp. */
  #define SGX_ENCLAVE_OFFSET_OF_RUN		16

@@ -27,6 +28,7 @@
  SYM_FUNC_START(__vdso_sgx_enter_enclave)
  	/* Prolog */
  	.cfi_startproc
+	ENDBR
  	push	%rbp
  	.cfi_adjust_cfa_offset	8
  	.cfi_rel_offset		%rbp, 0

@@ -62,6 +64,7 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
  .Lasync_exit_pointer:
  .Lenclu_eenter_eresume:
  	enclu
+	ENDBR
  
  	/* EEXIT jumps here unless the enclave is doing something fancy. */
  	mov	SGX_ENCLAVE_OFFSET_OF_RUN(%rbp), %rbx

@@ -91,6 +94,7 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
  	jmp	.Lout
  
  .Lhandle_exception:
+	ENDBR
  	mov	SGX_ENCLAVE_OFFSET_OF_RUN(%rbp), %rbx
  
  	/* Set the exception info. */

-- 
2.21.0

Looks good to me.

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Thanks for reviewing.  In response to Dave's and Boris' comments, I will 
replace ENDBR macro with _CET_ENDBR that comes from the compiler.  Can I 
still keep the Reviewed-by?

/Jarkko

On Tue, Mar 16, 2021 at 12:27:19PM -0700, Yu, Yu-cheng wrote:

On 3/16/2021 12:22 PM, Jarkko Sakkinen wrote:

quoted

On Tue, Mar 16, 2021 at 08:13:19AM -0700, Yu-cheng Yu wrote:

quoted

ENDBR is a special new instruction for the Indirect Branch Tracking (IBT)
component of CET.  IBT prevents attacks by ensuring that (most) indirect
branches and function calls may only land at ENDBR instructions.  Branches
that don't follow the rules will result in control flow (#CF) exceptions.

ENDBR is a noop when IBT is unsupported or disabled.  Most ENDBR
instructions are inserted automatically by the compiler, but branch
targets written in assembly must have ENDBR added manually.

Add ENDBR to __vdso_sgx_enter_enclave() branch targets.

Signed-off-by: Yu-cheng Yu <redacted>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
  arch/x86/entry/vdso/vsgx.S | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/arch/x86/entry/vdso/vsgx.S b/arch/x86/entry/vdso/vsgx.S
index 86a0e94f68df..1baa9b49053e 100644
--- a/arch/x86/entry/vdso/vsgx.S
+++ b/arch/x86/entry/vdso/vsgx.S

@@ -6,6 +6,7 @@
  #include <asm/enclu.h>
  #include "extable.h"
+#include "../calling.h"
  /* Relative to %rbp. */
  #define SGX_ENCLAVE_OFFSET_OF_RUN		16

@@ -27,6 +28,7 @@
  SYM_FUNC_START(__vdso_sgx_enter_enclave)
  	/* Prolog */
  	.cfi_startproc
+	ENDBR
  	push	%rbp
  	.cfi_adjust_cfa_offset	8
  	.cfi_rel_offset		%rbp, 0

@@ -62,6 +64,7 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
  .Lasync_exit_pointer:
  .Lenclu_eenter_eresume:
  	enclu
+	ENDBR
  	/* EEXIT jumps here unless the enclave is doing something fancy. */
  	mov	SGX_ENCLAVE_OFFSET_OF_RUN(%rbp), %rbx

@@ -91,6 +94,7 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
  	jmp	.Lout
  .Lhandle_exception:
+	ENDBR
  	mov	SGX_ENCLAVE_OFFSET_OF_RUN(%rbp), %rbx
  	/* Set the exception info. */

-- 
2.21.0

Looks good to me.

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Thanks for reviewing.  In response to Dave's and Boris' comments, I will
replace ENDBR macro with _CET_ENDBR that comes from the compiler.  Can I
still keep the Reviewed-by?

I'll rather re-ack, thanks. Most likely give reviewed-by but I always
prefer to see the code change before doing that.

/Jarkko