* [PATCH v22 1/8] x86/cet/ibt: Update Kconfig for user-mode Indirect Branch Tracking
2021-03-10 22:05 [PATCH v22 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
@ 2021-03-10 22:05 ` Yu-cheng Yu
2021-03-10 22:05 ` [PATCH v22 2/8] x86/cet/ibt: User-mode Indirect Branch Tracking support Yu-cheng Yu
` (6 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Yu-cheng Yu @ 2021-03-10 22:05 UTC (permalink / raw)
To: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
Cc: Yu-cheng Yu
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 <yu-cheng.yu@intel.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v22 2/8] x86/cet/ibt: User-mode Indirect Branch Tracking support
2021-03-10 22:05 [PATCH v22 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
2021-03-10 22:05 ` [PATCH v22 1/8] x86/cet/ibt: Update Kconfig for user-mode " Yu-cheng Yu
@ 2021-03-10 22:05 ` Yu-cheng Yu
2021-03-10 22:05 ` [PATCH v22 3/8] x86/cet/ibt: Handle signals for Indirect Branch Tracking Yu-cheng Yu
` (5 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Yu-cheng Yu @ 2021-03-10 22:05 UTC (permalink / raw)
To: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
Cc: Yu-cheng Yu
Introduce user-mode Indirect Branch Tracking (IBT) support. Add routines
for the setup/disable of IBT.
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v22 3/8] x86/cet/ibt: Handle signals for Indirect Branch Tracking
2021-03-10 22:05 [PATCH v22 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
2021-03-10 22:05 ` [PATCH v22 1/8] x86/cet/ibt: Update Kconfig for user-mode " Yu-cheng Yu
2021-03-10 22:05 ` [PATCH v22 2/8] x86/cet/ibt: User-mode Indirect Branch Tracking support Yu-cheng Yu
@ 2021-03-10 22:05 ` Yu-cheng Yu
2021-03-10 22:05 ` [PATCH v22 4/8] x86/cet/ibt: Update ELF header parsing " Yu-cheng Yu
` (4 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Yu-cheng Yu @ 2021-03-10 22:05 UTC (permalink / raw)
To: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
Cc: Yu-cheng Yu
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 <yu-cheng.yu@intel.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v22 4/8] x86/cet/ibt: Update ELF header parsing for Indirect Branch Tracking
2021-03-10 22:05 [PATCH v22 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
` (2 preceding siblings ...)
2021-03-10 22:05 ` [PATCH v22 3/8] x86/cet/ibt: Handle signals for Indirect Branch Tracking Yu-cheng Yu
@ 2021-03-10 22:05 ` Yu-cheng Yu
2021-03-10 22:05 ` [PATCH v22 5/8] x86/cet/ibt: Update arch_prctl functions " Yu-cheng Yu
` (3 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Yu-cheng Yu @ 2021-03-10 22:05 UTC (permalink / raw)
To: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
Cc: Yu-cheng Yu
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 <yu-cheng.yu@intel.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v22 5/8] x86/cet/ibt: Update arch_prctl functions for Indirect Branch Tracking
2021-03-10 22:05 [PATCH v22 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
` (3 preceding siblings ...)
2021-03-10 22:05 ` [PATCH v22 4/8] x86/cet/ibt: Update ELF header parsing " Yu-cheng Yu
@ 2021-03-10 22:05 ` Yu-cheng Yu
2021-03-10 22:05 ` [PATCH v22 6/8] x86/vdso/32: Add ENDBR32 to __kernel_vsyscall entry point Yu-cheng Yu
` (2 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Yu-cheng Yu @ 2021-03-10 22:05 UTC (permalink / raw)
To: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
Cc: Yu-cheng Yu
From: "H.J. Lu" <hjl.tools@gmail.com>
Update ARCH_X86_CET_STATUS and ARCH_X86_CET_DISABLE for Indirect Branch
Tracking.
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v22 6/8] x86/vdso/32: Add ENDBR32 to __kernel_vsyscall entry point
2021-03-10 22:05 [PATCH v22 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
` (4 preceding siblings ...)
2021-03-10 22:05 ` [PATCH v22 5/8] x86/cet/ibt: Update arch_prctl functions " Yu-cheng Yu
@ 2021-03-10 22:05 ` Yu-cheng Yu
2021-03-10 22:05 ` [PATCH v22 7/8] x86/vdso: Insert endbr32/endbr64 to vDSO Yu-cheng Yu
2021-03-10 22:05 ` [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave Yu-cheng Yu
7 siblings, 0 replies; 21+ messages in thread
From: Yu-cheng Yu @ 2021-03-10 22:05 UTC (permalink / raw)
To: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
Cc: Yu-cheng Yu
From: "H.J. Lu" <hjl.tools@gmail.com>
Add ENDBR32 to __kernel_vsyscall entry point.
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Acked-by: Andy Lutomirski <luto@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
arch/x86/entry/vdso/vdso32/system_call.S | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/entry/vdso/vdso32/system_call.S b/arch/x86/entry/vdso/vdso32/system_call.S
index de1fff7188aa..f19eaec3de3b 100644
--- a/arch/x86/entry/vdso/vdso32/system_call.S
+++ b/arch/x86/entry/vdso/vdso32/system_call.S
@@ -14,6 +14,9 @@
ALIGN
__kernel_vsyscall:
CFI_STARTPROC
+#ifdef CONFIG_X86_CET
+ endbr32
+#endif
/*
* Reshuffle regs so that all of any of the entry instructions
* will preserve enough state.
--
2.21.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v22 7/8] x86/vdso: Insert endbr32/endbr64 to vDSO
2021-03-10 22:05 [PATCH v22 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
` (5 preceding siblings ...)
2021-03-10 22:05 ` [PATCH v22 6/8] x86/vdso/32: Add ENDBR32 to __kernel_vsyscall entry point Yu-cheng Yu
@ 2021-03-10 22:05 ` Yu-cheng Yu
2021-03-10 22:05 ` [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave Yu-cheng Yu
7 siblings, 0 replies; 21+ messages in thread
From: Yu-cheng Yu @ 2021-03-10 22:05 UTC (permalink / raw)
To: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
Cc: Yu-cheng Yu
From: "H.J. Lu" <hjl.tools@gmail.com>
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 <hjl.tools@gmail.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Acked-by: Andy Lutomirski <luto@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-10 22:05 [PATCH v22 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
` (6 preceding siblings ...)
2021-03-10 22:05 ` [PATCH v22 7/8] x86/vdso: Insert endbr32/endbr64 to vDSO Yu-cheng Yu
@ 2021-03-10 22:05 ` Yu-cheng Yu
2021-03-10 22:39 ` Jarkko Sakkinen
7 siblings, 1 reply; 21+ messages in thread
From: Yu-cheng Yu @ 2021-03-10 22:05 UTC (permalink / raw)
To: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
Cc: Yu-cheng Yu, Jarkko Sakkinen
When CET is enabled, __vdso_sgx_enter_enclave() needs an endbr64
in the beginning of the function.
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
---
arch/x86/entry/vdso/vsgx.S | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/entry/vdso/vsgx.S b/arch/x86/entry/vdso/vsgx.S
index 86a0e94f68df..a70d4d09f713 100644
--- a/arch/x86/entry/vdso/vsgx.S
+++ b/arch/x86/entry/vdso/vsgx.S
@@ -27,6 +27,9 @@
SYM_FUNC_START(__vdso_sgx_enter_enclave)
/* Prolog */
.cfi_startproc
+#ifdef CONFIG_X86_CET
+ endbr64
+#endif
push %rbp
.cfi_adjust_cfa_offset 8
.cfi_rel_offset %rbp, 0
--
2.21.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-10 22:05 ` [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave Yu-cheng Yu
@ 2021-03-10 22:39 ` Jarkko Sakkinen
2021-03-10 22:55 ` Yu, Yu-cheng
0 siblings, 1 reply; 21+ messages in thread
From: Jarkko Sakkinen @ 2021-03-10 22:39 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
On Wed, Mar 10, 2021 at 02:05:19PM -0800, Yu-cheng Yu wrote:
> When CET is enabled, __vdso_sgx_enter_enclave() needs an endbr64
> in the beginning of the function.
OK.
What you should do is to explain what it does and why it's needed.
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> arch/x86/entry/vdso/vsgx.S | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/entry/vdso/vsgx.S b/arch/x86/entry/vdso/vsgx.S
> index 86a0e94f68df..a70d4d09f713 100644
> --- a/arch/x86/entry/vdso/vsgx.S
> +++ b/arch/x86/entry/vdso/vsgx.S
> @@ -27,6 +27,9 @@
> SYM_FUNC_START(__vdso_sgx_enter_enclave)
> /* Prolog */
> .cfi_startproc
> +#ifdef CONFIG_X86_CET
> + endbr64
> +#endif
> push %rbp
> .cfi_adjust_cfa_offset 8
> .cfi_rel_offset %rbp, 0
> --
> 2.21.0
>
>
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-10 22:39 ` Jarkko Sakkinen
@ 2021-03-10 22:55 ` Yu, Yu-cheng
2021-03-10 23:17 ` Borislav Petkov
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Yu, Yu-cheng @ 2021-03-10 22:55 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
On 3/10/2021 2:39 PM, Jarkko Sakkinen wrote:
> On Wed, Mar 10, 2021 at 02:05:19PM -0800, Yu-cheng Yu wrote:
>> When CET is enabled, __vdso_sgx_enter_enclave() needs an endbr64
>> in the beginning of the function.
>
> OK.
>
> What you should do is to explain what it does and why it's needed.
>
The endbr marks a branch target. Without the "no-track" prefix, if an
indirect call/jmp reaches a non-endbr opcode, a control-protection fault
is raised. Usually endbr's are inserted by the compiler. For assembly,
these have to be put in manually. I will add this in the commit log if
there is another revision. Thanks!
--
Yu-cheng
>>
>> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
>> Cc: Andy Lutomirski <luto@kernel.org>
>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> Cc: Jarkko Sakkinen <jarkko@kernel.org>
>> ---
>> arch/x86/entry/vdso/vsgx.S | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/x86/entry/vdso/vsgx.S b/arch/x86/entry/vdso/vsgx.S
>> index 86a0e94f68df..a70d4d09f713 100644
>> --- a/arch/x86/entry/vdso/vsgx.S
>> +++ b/arch/x86/entry/vdso/vsgx.S
>> @@ -27,6 +27,9 @@
>> SYM_FUNC_START(__vdso_sgx_enter_enclave)
>> /* Prolog */
>> .cfi_startproc
>> +#ifdef CONFIG_X86_CET
>> + endbr64
>> +#endif
>> push %rbp
>> .cfi_adjust_cfa_offset 8
>> .cfi_rel_offset %rbp, 0
>> --
>> 2.21.0
>>
>>
>
> /Jarkko
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-10 22:55 ` Yu, Yu-cheng
@ 2021-03-10 23:17 ` Borislav Petkov
2021-03-10 23:20 ` Dave Hansen
2021-03-11 3:36 ` Jarkko Sakkinen
2 siblings, 0 replies; 21+ messages in thread
From: Borislav Petkov @ 2021-03-10 23:17 UTC (permalink / raw)
To: Yu, Yu-cheng
Cc: Jarkko Sakkinen, x86, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, linux-kernel, linux-doc, linux-mm, linux-arch,
linux-api, Arnd Bergmann, Andy Lutomirski, Balbir Singh,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin, Weijiang Yang, Pengfei Xu,
Haitao Huang
On Wed, Mar 10, 2021 at 02:55:55PM -0800, Yu, Yu-cheng wrote:
> > > @@ -27,6 +27,9 @@
> > > SYM_FUNC_START(__vdso_sgx_enter_enclave)
> > > /* Prolog */
> > > .cfi_startproc
> > > +#ifdef CONFIG_X86_CET
> > > + endbr64
> > > +#endif
You can hide this ifdeffery in a macro and have
ENDBR64
at the callsite and define
.macro ENDBR64
#ifdef CONFIG_X86_CET
endbr64
#endif
.endm
or so, perhaps. Ditto for endbr32.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-10 22:55 ` Yu, Yu-cheng
2021-03-10 23:17 ` Borislav Petkov
@ 2021-03-10 23:20 ` Dave Hansen
2021-03-10 23:22 ` Yu, Yu-cheng
2021-03-12 16:55 ` Jarkko Sakkinen
2021-03-11 3:36 ` Jarkko Sakkinen
2 siblings, 2 replies; 21+ messages in thread
From: Dave Hansen @ 2021-03-10 23:20 UTC (permalink / raw)
To: Yu, Yu-cheng, Jarkko Sakkinen
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
On 3/10/21 2:55 PM, Yu, Yu-cheng wrote:
> On 3/10/2021 2:39 PM, Jarkko Sakkinen wrote:
>> On Wed, Mar 10, 2021 at 02:05:19PM -0800, Yu-cheng Yu wrote:
>>> When CET is enabled, __vdso_sgx_enter_enclave() needs an endbr64
>>> in the beginning of the function.
>>
>> OK.
>>
>> What you should do is to explain what it does and why it's needed.
>>
>
> The endbr marks a branch target. Without the "no-track" prefix, if an
> indirect call/jmp reaches a non-endbr opcode, a control-protection fault
> is raised. Usually endbr's are inserted by the compiler. For assembly,
> these have to be put in manually. I will add this in the commit log if
> there is another revision. Thanks!
This is close, but it's missing a detail or two that I think is
important for someone like Jarkko trying to figure out what it means for
his subsystem or driver.
I'd probably say:
ENDBR is a special new instruction for the Indirect Branch Tracking
(IBR) 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, like this one.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-10 23:20 ` Dave Hansen
@ 2021-03-10 23:22 ` Yu, Yu-cheng
2021-03-12 16:55 ` Jarkko Sakkinen
1 sibling, 0 replies; 21+ messages in thread
From: Yu, Yu-cheng @ 2021-03-10 23:22 UTC (permalink / raw)
To: Dave Hansen, Jarkko Sakkinen
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
On 3/10/2021 3:20 PM, Dave Hansen wrote:
> On 3/10/21 2:55 PM, Yu, Yu-cheng wrote:
>> On 3/10/2021 2:39 PM, Jarkko Sakkinen wrote:
>>> On Wed, Mar 10, 2021 at 02:05:19PM -0800, Yu-cheng Yu wrote:
>>>> When CET is enabled, __vdso_sgx_enter_enclave() needs an endbr64
>>>> in the beginning of the function.
>>>
>>> OK.
>>>
>>> What you should do is to explain what it does and why it's needed.
>>>
>>
>> The endbr marks a branch target. Without the "no-track" prefix, if an
>> indirect call/jmp reaches a non-endbr opcode, a control-protection fault
>> is raised. Usually endbr's are inserted by the compiler. For assembly,
>> these have to be put in manually. I will add this in the commit log if
>> there is another revision. Thanks!
>
> This is close, but it's missing a detail or two that I think is
> important for someone like Jarkko trying to figure out what it means for
> his subsystem or driver.
>
> I'd probably say:
>
> ENDBR is a special new instruction for the Indirect Branch Tracking
> (IBR) 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, like this one.
>
Ok, I will update. Thanks!
--
Yu-cheng
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-10 23:20 ` Dave Hansen
2021-03-10 23:22 ` Yu, Yu-cheng
@ 2021-03-12 16:55 ` Jarkko Sakkinen
2021-03-12 16:57 ` Jarkko Sakkinen
2021-03-12 16:59 ` Dave Hansen
1 sibling, 2 replies; 21+ messages in thread
From: Jarkko Sakkinen @ 2021-03-12 16:55 UTC (permalink / raw)
To: Dave Hansen
Cc: Yu, Yu-cheng, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin, Weijiang Yang, Pengfei Xu,
Haitao Huang
On Wed, Mar 10, 2021 at 03:20:20PM -0800, Dave Hansen wrote:
> On 3/10/21 2:55 PM, Yu, Yu-cheng wrote:
> > On 3/10/2021 2:39 PM, Jarkko Sakkinen wrote:
> >> On Wed, Mar 10, 2021 at 02:05:19PM -0800, Yu-cheng Yu wrote:
> >>> When CET is enabled, __vdso_sgx_enter_enclave() needs an endbr64
> >>> in the beginning of the function.
> >>
> >> OK.
> >>
> >> What you should do is to explain what it does and why it's needed.
> >>
> >
> > The endbr marks a branch target. Without the "no-track" prefix, if an
> > indirect call/jmp reaches a non-endbr opcode, a control-protection fault
> > is raised. Usually endbr's are inserted by the compiler. For assembly,
> > these have to be put in manually. I will add this in the commit log if
> > there is another revision. Thanks!
>
> This is close, but it's missing a detail or two that I think is
> important for someone like Jarkko trying to figure out what it means for
> his subsystem or driver.
>
> I'd probably say:
>
> ENDBR is a special new instruction for the Indirect Branch Tracking
> (IBR) 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, like this one.
Thank you, this clears the whole thing a lot.
Doesn't this mean that it could be there just as well unconditionally?
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-12 16:55 ` Jarkko Sakkinen
@ 2021-03-12 16:57 ` Jarkko Sakkinen
2021-03-12 16:59 ` Dave Hansen
1 sibling, 0 replies; 21+ messages in thread
From: Jarkko Sakkinen @ 2021-03-12 16:57 UTC (permalink / raw)
To: Dave Hansen
Cc: Yu, Yu-cheng, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin, Weijiang Yang, Pengfei Xu,
Haitao Huang
On Fri, Mar 12, 2021 at 06:55:57PM +0200, Jarkko Sakkinen wrote:
> On Wed, Mar 10, 2021 at 03:20:20PM -0800, Dave Hansen wrote:
> > On 3/10/21 2:55 PM, Yu, Yu-cheng wrote:
> > > On 3/10/2021 2:39 PM, Jarkko Sakkinen wrote:
> > >> On Wed, Mar 10, 2021 at 02:05:19PM -0800, Yu-cheng Yu wrote:
> > >>> When CET is enabled, __vdso_sgx_enter_enclave() needs an endbr64
> > >>> in the beginning of the function.
> > >>
> > >> OK.
> > >>
> > >> What you should do is to explain what it does and why it's needed.
> > >>
> > >
> > > The endbr marks a branch target. Without the "no-track" prefix, if an
> > > indirect call/jmp reaches a non-endbr opcode, a control-protection fault
> > > is raised. Usually endbr's are inserted by the compiler. For assembly,
> > > these have to be put in manually. I will add this in the commit log if
> > > there is another revision. Thanks!
> >
> > This is close, but it's missing a detail or two that I think is
> > important for someone like Jarkko trying to figure out what it means for
> > his subsystem or driver.
> >
> > I'd probably say:
> >
> > ENDBR is a special new instruction for the Indirect Branch Tracking
> > (IBR) 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, like this one.
>
> Thank you, this clears the whole thing a lot.
>
> Doesn't this mean that it could be there just as well unconditionally?
Please, ignore the question (got the answer).
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-12 16:55 ` Jarkko Sakkinen
2021-03-12 16:57 ` Jarkko Sakkinen
@ 2021-03-12 16:59 ` Dave Hansen
1 sibling, 0 replies; 21+ messages in thread
From: Dave Hansen @ 2021-03-12 16:59 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Yu, Yu-cheng, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin, Weijiang Yang, Pengfei Xu,
Haitao Huang
On 3/12/21 8:55 AM, Jarkko Sakkinen wrote:
>> 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, like this one.
> Thank you, this clears the whole thing a lot.
>
> Doesn't this mean that it could be there just as well unconditionally?
It could be there unconditionally. But, I think it's still worth the
#ifdef just out of the principle of being as tidy as possible. The
#ifdef is basically as low cost and low complexity as you get. It is
also somewhat self-documenting: "This instruction is only necessary when
your CPU supports IBT".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-10 22:55 ` Yu, Yu-cheng
2021-03-10 23:17 ` Borislav Petkov
2021-03-10 23:20 ` Dave Hansen
@ 2021-03-11 3:36 ` Jarkko Sakkinen
2021-03-11 8:42 ` Peter Zijlstra
2 siblings, 1 reply; 21+ messages in thread
From: Jarkko Sakkinen @ 2021-03-11 3:36 UTC (permalink / raw)
To: Yu, Yu-cheng
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang,
Pengfei Xu, Haitao Huang
On Wed, Mar 10, 2021 at 02:55:55PM -0800, Yu, Yu-cheng wrote:
> On 3/10/2021 2:39 PM, Jarkko Sakkinen wrote:
> > On Wed, Mar 10, 2021 at 02:05:19PM -0800, Yu-cheng Yu wrote:
> > > When CET is enabled, __vdso_sgx_enter_enclave() needs an endbr64
> > > in the beginning of the function.
> >
> > OK.
> >
> > What you should do is to explain what it does and why it's needed.
> >
>
> The endbr marks a branch target. Without the "no-track" prefix, if an
> indirect call/jmp reaches a non-endbr opcode, a control-protection fault is
> raised. Usually endbr's are inserted by the compiler. For assembly, these
> have to be put in manually. I will add this in the commit log if there is
> another revision. Thanks!
Thanks for the explanation. There is another revision, because this is
lacking from the commit message.
Does it do any harm to put it there unconditionally?
>
> --
> Yu-cheng
>
> > >
> > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > Cc: Andy Lutomirski <luto@kernel.org>
> > > Cc: Dave Hansen <dave.hansen@linux.intel.com>
> > > Cc: Jarkko Sakkinen <jarkko@kernel.org>
> > > ---
> > > arch/x86/entry/vdso/vsgx.S | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/arch/x86/entry/vdso/vsgx.S b/arch/x86/entry/vdso/vsgx.S
> > > index 86a0e94f68df..a70d4d09f713 100644
> > > --- a/arch/x86/entry/vdso/vsgx.S
> > > +++ b/arch/x86/entry/vdso/vsgx.S
> > > @@ -27,6 +27,9 @@
> > > SYM_FUNC_START(__vdso_sgx_enter_enclave)
> > > /* Prolog */
> > > .cfi_startproc
> > > +#ifdef CONFIG_X86_CET
> > > + endbr64
> > > +#endif
> > > push %rbp
> > > .cfi_adjust_cfa_offset 8
> > > .cfi_rel_offset %rbp, 0
> > > --
> > > 2.21.0
> > >
> > >
> >
> > /Jarkko
> >
>
>
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-11 3:36 ` Jarkko Sakkinen
@ 2021-03-11 8:42 ` Peter Zijlstra
2021-03-11 15:44 ` Yu, Yu-cheng
2021-03-12 16:56 ` Jarkko Sakkinen
0 siblings, 2 replies; 21+ messages in thread
From: Peter Zijlstra @ 2021-03-11 8:42 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Yu, Yu-cheng, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin,
Weijiang Yang, Pengfei Xu, Haitao Huang
On Thu, Mar 11, 2021 at 05:36:06AM +0200, Jarkko Sakkinen wrote:
> Does it do any harm to put it there unconditionally?
Blows up your text footprint and I$ pressure. These instructions are 4
bytes each.
Aside from that, they're a NOP, so only consume front-end resources
(hopefully) on older CPUs and when IBT is disabled.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-11 8:42 ` Peter Zijlstra
@ 2021-03-11 15:44 ` Yu, Yu-cheng
2021-03-12 16:56 ` Jarkko Sakkinen
1 sibling, 0 replies; 21+ messages in thread
From: Yu, Yu-cheng @ 2021-03-11 15:44 UTC (permalink / raw)
To: Peter Zijlstra, Jarkko Sakkinen
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin, Weijiang Yang, Pengfei Xu,
Haitao Huang
On 3/11/2021 12:42 AM, Peter Zijlstra wrote:
> On Thu, Mar 11, 2021 at 05:36:06AM +0200, Jarkko Sakkinen wrote:
>> Does it do any harm to put it there unconditionally?
>
> Blows up your text footprint and I$ pressure. These instructions are 4
> bytes each.
>
> Aside from that, they're a NOP, so only consume front-end resources
> (hopefully) on older CPUs and when IBT is disabled.
>
Thanks Peter. I think probably we'll do the macro Boris suggested.
That takes care of the visual clutter, and eliminates the need of using
.byte when the assembler is outdated.
--
Yu-cheng
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v22 8/8] x86/vdso: Add ENDBR64 to __vdso_sgx_enter_enclave
2021-03-11 8:42 ` Peter Zijlstra
2021-03-11 15:44 ` Yu, Yu-cheng
@ 2021-03-12 16:56 ` Jarkko Sakkinen
1 sibling, 0 replies; 21+ messages in thread
From: Jarkko Sakkinen @ 2021-03-12 16:56 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Yu, Yu-cheng, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin,
Weijiang Yang, Pengfei Xu, Haitao Huang
On Thu, Mar 11, 2021 at 09:42:05AM +0100, Peter Zijlstra wrote:
> On Thu, Mar 11, 2021 at 05:36:06AM +0200, Jarkko Sakkinen wrote:
> > Does it do any harm to put it there unconditionally?
>
> Blows up your text footprint and I$ pressure. These instructions are 4
> bytes each.
>
> Aside from that, they're a NOP, so only consume front-end resources
> (hopefully) on older CPUs and when IBT is disabled.
OK, understood, thanks for the explanation.
/Jarkko
^ permalink raw reply [flat|nested] 21+ messages in thread