linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v12 0/8] Control-flow Enforcement: Indirect Branch Tracking
@ 2020-09-18 19:23 Yu-cheng Yu
  2020-09-18 19:23 ` [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode " Yu-cheng Yu
                   ` (5 more replies)
  0 siblings, 6 replies; 39+ messages in thread
From: Yu-cheng Yu @ 2020-09-18 19:23 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
  Cc: Yu-cheng Yu

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 v12:

- Replace obj file list with $(vobjs) $(vobjs32) in VDSO Makefile.
- Disable vsyscall emulation only when it is attempted.
- Split out ptrace patch.

[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 v11.

    https://lkml.kernel.org/r/20200825002645.3658-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 ENDBR32 to __kernel_vsyscall entry point
  x86/vdso: Insert endbr32/endbr64 to vDSO

Yu-cheng Yu (5):
  x86/cet/ibt: Add Kconfig option 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: ELF header parsing for Indirect Branch Tracking
  x86: Disallow vsyscall emulation when CET is enabled

 arch/x86/Kconfig                              | 18 ++++++
 arch/x86/entry/vdso/Makefile                  |  4 ++
 arch/x86/entry/vdso/vdso32/system_call.S      |  3 +
 arch/x86/entry/vsyscall/vsyscall_64.c         |  9 +++
 arch/x86/include/asm/cet.h                    |  3 +
 arch/x86/include/asm/disabled-features.h      |  8 ++-
 arch/x86/kernel/cet.c                         | 60 ++++++++++++++++++-
 arch/x86/kernel/cet_prctl.c                   |  8 ++-
 arch/x86/kernel/cpu/common.c                  | 17 ++++++
 arch/x86/kernel/fpu/signal.c                  |  8 ++-
 arch/x86/kernel/process_64.c                  |  8 +++
 .../arch/x86/include/asm/disabled-features.h  |  8 ++-
 12 files changed, 146 insertions(+), 8 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-18 19:23 [PATCH v12 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
@ 2020-09-18 19:23 ` Yu-cheng Yu
  2020-09-18 20:24   ` Randy Dunlap
  2020-09-18 19:23 ` [PATCH v12 4/8] x86/cet/ibt: ELF header parsing for " Yu-cheng Yu
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 39+ messages in thread
From: Yu-cheng Yu @ 2020-09-18 19:23 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
  Cc: Yu-cheng Yu

Introduce Kconfig option X86_INTEL_BRANCH_TRACKING_USER.

Indirect Branch Tracking (IBT) provides protection against CALL-/JMP-
oriented programming attacks.  It is active when the kernel has this
feature enabled, and the processor and the application support it.
When this feature is enabled, legacy non-IBT applications continue to
work, but without IBT protection.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
v10:
- Change build-time CET check to config depends on.

 arch/x86/Kconfig | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 6b6dad011763..b047e0a8d1c2 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1963,6 +1963,22 @@ config X86_INTEL_SHADOW_STACK_USER
 
 	  If unsure, say y.
 
+config X86_INTEL_BRANCH_TRACKING_USER
+	prompt "Intel Indirect Branch Tracking for user-mode"
+	def_bool n
+	depends on CPU_SUP_INTEL && X86_64
+	depends on $(cc-option,-fcf-protection)
+	select X86_INTEL_CET
+	help
+	  Indirect Branch Tracking (IBT) provides protection against
+	  CALL-/JMP-oriented programming attacks.  It is active when
+	  the kernel has this feature enabled, and the processor and
+	  the application support it.  When this feature is enabled,
+	  legacy non-IBT applications continue to work, but without
+	  IBT protection.
+
+	  If unsure, say y
+
 config EFI
 	bool "EFI runtime service support"
 	depends on ACPI
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v12 4/8] x86/cet/ibt: ELF header parsing for Indirect Branch Tracking
  2020-09-18 19:23 [PATCH v12 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
  2020-09-18 19:23 ` [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode " Yu-cheng Yu
@ 2020-09-18 19:23 ` Yu-cheng Yu
  2020-09-18 19:23 ` [PATCH v12 5/8] x86/cet/ibt: Update arch_prctl functions " Yu-cheng Yu
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 39+ messages in thread
From: Yu-cheng Yu @ 2020-09-18 19:23 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
  Cc: Yu-cheng Yu

Update arch_setup_elf_property() for Indirect Branch Tracking.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
v9:
- Change cpu_feature_enabled() to static_cpu_has().

 arch/x86/Kconfig             | 2 ++
 arch/x86/kernel/process_64.c | 8 ++++++++
 2 files changed, 10 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b047e0a8d1c2..5bd6d6a10047 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1969,6 +1969,8 @@ config X86_INTEL_BRANCH_TRACKING_USER
 	depends on CPU_SUP_INTEL && X86_64
 	depends on $(cc-option,-fcf-protection)
 	select X86_INTEL_CET
+	select ARCH_USE_GNU_PROPERTY
+	select ARCH_BINFMT_ELF_STATE
 	help
 	  Indirect Branch Tracking (IBT) provides protection against
 	  CALL-/JMP-oriented programming attacks.  It is active when
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index fd4644865a3b..c084e1a37d11 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -866,6 +866,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] 39+ messages in thread

* [PATCH v12 5/8] x86/cet/ibt: Update arch_prctl functions for Indirect Branch Tracking
  2020-09-18 19:23 [PATCH v12 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
  2020-09-18 19:23 ` [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode " Yu-cheng Yu
  2020-09-18 19:23 ` [PATCH v12 4/8] x86/cet/ibt: ELF header parsing for " Yu-cheng Yu
@ 2020-09-18 19:23 ` Yu-cheng Yu
  2020-09-18 19:23 ` [PATCH v12 6/8] x86/vdso/32: Add ENDBR32 to __kernel_vsyscall entry point Yu-cheng Yu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 39+ messages in thread
From: Yu-cheng Yu @ 2020-09-18 19:23 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
  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>
---
 arch/x86/kernel/cet_prctl.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cet_prctl.c b/arch/x86/kernel/cet_prctl.c
index 3e1d049e65c3..78631c5d944e 100644
--- a/arch/x86/kernel/cet_prctl.c
+++ b/arch/x86/kernel/cet_prctl.c
@@ -22,6 +22,9 @@ static int copy_status_to_user(struct cet_status *cet, u64 arg2)
 		buf[2] = (u64)cet->shstk_size;
 	}
 
+	if (cet->ibt_enabled)
+		buf[0] |= GNU_PROPERTY_X86_FEATURE_1_IBT;
+
 	return copy_to_user((u64 __user *)arg2, buf, sizeof(buf));
 }
 
@@ -42,7 +45,8 @@ int prctl_cet(int option, u64 arg2)
 	if (option == ARCH_X86_CET_STATUS)
 		return copy_status_to_user(cet, arg2);
 
-	if (!static_cpu_has(X86_FEATURE_SHSTK))
+	if (!static_cpu_has(X86_FEATURE_SHSTK) &&
+	    !static_cpu_has(X86_FEATURE_IBT))
 		return -EOPNOTSUPP;
 
 	switch (option) {
@@ -56,6 +60,8 @@ int prctl_cet(int option, u64 arg2)
 			return -EINVAL;
 		if (features & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
 			cet_disable_shstk();
+		if (features & 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] 39+ messages in thread

* [PATCH v12 6/8] x86/vdso/32: Add ENDBR32 to __kernel_vsyscall entry point
  2020-09-18 19:23 [PATCH v12 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
                   ` (2 preceding siblings ...)
  2020-09-18 19:23 ` [PATCH v12 5/8] x86/cet/ibt: Update arch_prctl functions " Yu-cheng Yu
@ 2020-09-18 19:23 ` Yu-cheng Yu
  2020-09-18 19:23 ` [PATCH v12 7/8] x86/vdso: Insert endbr32/endbr64 to vDSO Yu-cheng Yu
  2020-09-18 19:23 ` [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled Yu-cheng Yu
  5 siblings, 0 replies; 39+ messages in thread
From: Yu-cheng Yu @ 2020-09-18 19:23 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
  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>
---
 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..5cf74ebd4746 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_INTEL_BRANCH_TRACKING_USER
+	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] 39+ messages in thread

* [PATCH v12 7/8] x86/vdso: Insert endbr32/endbr64 to vDSO
  2020-09-18 19:23 [PATCH v12 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
                   ` (3 preceding siblings ...)
  2020-09-18 19:23 ` [PATCH v12 6/8] x86/vdso/32: Add ENDBR32 to __kernel_vsyscall entry point Yu-cheng Yu
@ 2020-09-18 19:23 ` Yu-cheng Yu
  2020-09-18 19:23 ` [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled Yu-cheng Yu
  5 siblings, 0 replies; 39+ messages in thread
From: Yu-cheng Yu @ 2020-09-18 19:23 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
  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>
---
v12:
- Replace object file list with $(vobjs) $(vobjs32).

 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 215376d975a2..3f8b5f513adb 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -94,6 +94,10 @@ endif
 
 $(vobjs): KBUILD_CFLAGS := $(filter-out $(GCC_PLUGINS_CFLAGS) $(RETPOLINE_CFLAGS),$(KBUILD_CFLAGS)) $(CFL)
 
+ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+$(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] 39+ messages in thread

* [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-18 19:23 [PATCH v12 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
                   ` (4 preceding siblings ...)
  2020-09-18 19:23 ` [PATCH v12 7/8] x86/vdso: Insert endbr32/endbr64 to vDSO Yu-cheng Yu
@ 2020-09-18 19:23 ` Yu-cheng Yu
  2020-09-18 19:32   ` Dave Hansen
  2020-09-19  0:11   ` Andy Lutomirski
  5 siblings, 2 replies; 39+ messages in thread
From: Yu-cheng Yu @ 2020-09-18 19:23 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
  Cc: Yu-cheng Yu

Emulation of the legacy vsyscall page is required by some programs
built before 2013.  Newer programs after 2013 don't use it.
Disable vsyscall emulation when Control-flow Enforcement (CET) is
enabled to enhance security.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
v12:
- Disable vsyscall emulation only when it is attempted (vs. at compile time).

 arch/x86/entry/vsyscall/vsyscall_64.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
index 44c33103a955..3196e963e365 100644
--- a/arch/x86/entry/vsyscall/vsyscall_64.c
+++ b/arch/x86/entry/vsyscall/vsyscall_64.c
@@ -150,6 +150,15 @@ bool emulate_vsyscall(unsigned long error_code,
 
 	WARN_ON_ONCE(address != regs->ip);
 
+#ifdef CONFIG_X86_INTEL_CET
+	if (current->thread.cet.shstk_size ||
+	    current->thread.cet.ibt_enabled) {
+		warn_bad_vsyscall(KERN_INFO, regs,
+				  "vsyscall attempted with cet enabled");
+		return false;
+	}
+#endif
+
 	if (vsyscall_mode == NONE) {
 		warn_bad_vsyscall(KERN_INFO, regs,
 				  "vsyscall attempted with vsyscall=none");
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-18 19:23 ` [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled Yu-cheng Yu
@ 2020-09-18 19:32   ` Dave Hansen
  2020-09-18 21:00     ` Pavel Machek
  2020-09-19  0:11   ` Andy Lutomirski
  1 sibling, 1 reply; 39+ messages in thread
From: Dave Hansen @ 2020-09-18 19:32 UTC (permalink / raw)
  To: Yu-cheng Yu, 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

On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
> Emulation of the legacy vsyscall page is required by some programs
> built before 2013.  Newer programs after 2013 don't use it.
> Disable vsyscall emulation when Control-flow Enforcement (CET) is
> enabled to enhance security.

How does this "enhance security"?

What is the connection between vsyscall emulation and CET?

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-18 19:23 ` [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode " Yu-cheng Yu
@ 2020-09-18 20:24   ` Randy Dunlap
  2020-09-18 20:59     ` Pavel Machek
  0 siblings, 1 reply; 39+ messages in thread
From: Randy Dunlap @ 2020-09-18 20:24 UTC (permalink / raw)
  To: Yu-cheng Yu, 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, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin,
	Weijiang Yang

Hi,

If you do another version of this:

On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
> Introduce Kconfig option X86_INTEL_BRANCH_TRACKING_USER.
> 
> Indirect Branch Tracking (IBT) provides protection against CALL-/JMP-
> oriented programming attacks.  It is active when the kernel has this
> feature enabled, and the processor and the application support it.
> When this feature is enabled, legacy non-IBT applications continue to
> work, but without IBT protection.
> 
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
> v10:
> - Change build-time CET check to config depends on.
> 
>  arch/x86/Kconfig | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 6b6dad011763..b047e0a8d1c2 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1963,6 +1963,22 @@ config X86_INTEL_SHADOW_STACK_USER
>  
>  	  If unsure, say y.
>  
> +config X86_INTEL_BRANCH_TRACKING_USER
> +	prompt "Intel Indirect Branch Tracking for user-mode"
> +	def_bool n
> +	depends on CPU_SUP_INTEL && X86_64
> +	depends on $(cc-option,-fcf-protection)
> +	select X86_INTEL_CET
> +	help
> +	  Indirect Branch Tracking (IBT) provides protection against
> +	  CALL-/JMP-oriented programming attacks.  It is active when
> +	  the kernel has this feature enabled, and the processor and
> +	  the application support it.  When this feature is enabled,
> +	  legacy non-IBT applications continue to work, but without
> +	  IBT protection.
> +
> +	  If unsure, say y

	  If unsure, say y.

> +
>  config EFI
>  	bool "EFI runtime service support"
>  	depends on ACPI
> 


-- 
~Randy


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-18 20:24   ` Randy Dunlap
@ 2020-09-18 20:59     ` Pavel Machek
  2020-09-18 21:08       ` H.J. Lu
  2020-09-18 21:25       ` Yu, Yu-cheng
  0 siblings, 2 replies; 39+ messages in thread
From: Pavel Machek @ 2020-09-18 20:59 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Yu-cheng Yu, 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, Peter Zijlstra,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

[-- Attachment #1: Type: text/plain, Size: 2199 bytes --]

On Fri 2020-09-18 13:24:13, Randy Dunlap wrote:
> Hi,
> 
> If you do another version of this:
> 
> On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
> > Introduce Kconfig option X86_INTEL_BRANCH_TRACKING_USER.
> > 
> > Indirect Branch Tracking (IBT) provides protection against CALL-/JMP-
> > oriented programming attacks.  It is active when the kernel has this
> > feature enabled, and the processor and the application support it.
> > When this feature is enabled, legacy non-IBT applications continue to
> > work, but without IBT protection.
> > 
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > ---
> > v10:
> > - Change build-time CET check to config depends on.
> > 
> >  arch/x86/Kconfig | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 6b6dad011763..b047e0a8d1c2 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -1963,6 +1963,22 @@ config X86_INTEL_SHADOW_STACK_USER
> >  
> >  	  If unsure, say y.
> >  
> > +config X86_INTEL_BRANCH_TRACKING_USER
> > +	prompt "Intel Indirect Branch Tracking for user-mode"
> > +	def_bool n
> > +	depends on CPU_SUP_INTEL && X86_64
> > +	depends on $(cc-option,-fcf-protection)
> > +	select X86_INTEL_CET
> > +	help
> > +	  Indirect Branch Tracking (IBT) provides protection against
> > +	  CALL-/JMP-oriented programming attacks.  It is active when
> > +	  the kernel has this feature enabled, and the processor and
> > +	  the application support it.  When this feature is enabled,
> > +	  legacy non-IBT applications continue to work, but without
> > +	  IBT protection.
> > +
> > +	  If unsure, say y
> 
> 	  If unsure, say y.

Actually, it would be "If unsure, say Y.", to be consistent with the
rest of the Kconfig.

But I wonder if Yes by default is good idea. Only very new CPUs will
support this, right? Are they even available at the market? Should the
help text say "if your CPU is Whatever Lake or newer, ...." :-) ?

Best regards,
									Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-18 19:32   ` Dave Hansen
@ 2020-09-18 21:00     ` Pavel Machek
  2020-09-18 21:06       ` H.J. Lu
  2020-09-18 21:21       ` Yu, Yu-cheng
  0 siblings, 2 replies; 39+ messages in thread
From: Pavel Machek @ 2020-09-18 21:00 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Yu-cheng Yu, 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, Peter Zijlstra,
	Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin,
	Weijiang Yang

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

On Fri 2020-09-18 12:32:57, Dave Hansen wrote:
> On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
> > Emulation of the legacy vsyscall page is required by some programs
> > built before 2013.  Newer programs after 2013 don't use it.
> > Disable vsyscall emulation when Control-flow Enforcement (CET) is
> > enabled to enhance security.
> 
> How does this "enhance security"?
> 
> What is the connection between vsyscall emulation and CET?

Boom.

We don't break compatibility by default, and you should not tell
people to enable CET by default if you plan to do this.

Best regards,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-18 21:00     ` Pavel Machek
@ 2020-09-18 21:06       ` H.J. Lu
  2020-09-18 21:17         ` Dave Hansen
  2020-09-18 21:21       ` Yu, Yu-cheng
  1 sibling, 1 reply; 39+ messages in thread
From: H.J. Lu @ 2020-09-18 21:06 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Dave Hansen, Yu-cheng Yu, the arch/x86 maintainers,
	H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
	Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
	Florian Weimer, Jann Horn, Jonathan Corbet, Kees Cook,
	Mike Kravetz, Nadav Amit, Oleg Nesterov, Peter Zijlstra,
	Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin,
	Weijiang Yang

On Fri, Sep 18, 2020 at 2:00 PM Pavel Machek <pavel@ucw.cz> wrote:
>
> On Fri 2020-09-18 12:32:57, Dave Hansen wrote:
> > On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
> > > Emulation of the legacy vsyscall page is required by some programs
> > > built before 2013.  Newer programs after 2013 don't use it.
> > > Disable vsyscall emulation when Control-flow Enforcement (CET) is
> > > enabled to enhance security.
> >
> > How does this "enhance security"?
> >
> > What is the connection between vsyscall emulation and CET?
>
> Boom.
>
> We don't break compatibility by default, and you should not tell
> people to enable CET by default if you plan to do this.
>

Nothing will be broken.   CET enabled applications don't use/need
vsyscall emulation.

-- 
H.J.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-18 20:59     ` Pavel Machek
@ 2020-09-18 21:08       ` H.J. Lu
  2020-09-18 21:24         ` Pavel Machek
  2020-09-18 21:25       ` Yu, Yu-cheng
  1 sibling, 1 reply; 39+ messages in thread
From: H.J. Lu @ 2020-09-18 21:08 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Randy Dunlap, Yu-cheng Yu, the arch/x86 maintainers,
	H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
	Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
	Florian Weimer, Jann Horn, Jonathan Corbet, Kees Cook,
	Mike Kravetz, Nadav Amit, Oleg Nesterov, Peter Zijlstra,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

On Fri, Sep 18, 2020 at 1:59 PM Pavel Machek <pavel@ucw.cz> wrote:
>
> On Fri 2020-09-18 13:24:13, Randy Dunlap wrote:
> > Hi,
> >
> > If you do another version of this:
> >
> > On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
> > > Introduce Kconfig option X86_INTEL_BRANCH_TRACKING_USER.
> > >
> > > Indirect Branch Tracking (IBT) provides protection against CALL-/JMP-
> > > oriented programming attacks.  It is active when the kernel has this
> > > feature enabled, and the processor and the application support it.
> > > When this feature is enabled, legacy non-IBT applications continue to
> > > work, but without IBT protection.
> > >
> > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > ---
> > > v10:
> > > - Change build-time CET check to config depends on.
> > >
> > >  arch/x86/Kconfig | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > >
> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > index 6b6dad011763..b047e0a8d1c2 100644
> > > --- a/arch/x86/Kconfig
> > > +++ b/arch/x86/Kconfig
> > > @@ -1963,6 +1963,22 @@ config X86_INTEL_SHADOW_STACK_USER
> > >
> > >       If unsure, say y.
> > >
> > > +config X86_INTEL_BRANCH_TRACKING_USER
> > > +   prompt "Intel Indirect Branch Tracking for user-mode"
> > > +   def_bool n
> > > +   depends on CPU_SUP_INTEL && X86_64
> > > +   depends on $(cc-option,-fcf-protection)
> > > +   select X86_INTEL_CET
> > > +   help
> > > +     Indirect Branch Tracking (IBT) provides protection against
> > > +     CALL-/JMP-oriented programming attacks.  It is active when
> > > +     the kernel has this feature enabled, and the processor and
> > > +     the application support it.  When this feature is enabled,
> > > +     legacy non-IBT applications continue to work, but without
> > > +     IBT protection.
> > > +
> > > +     If unsure, say y
> >
> >         If unsure, say y.
>
> Actually, it would be "If unsure, say Y.", to be consistent with the
> rest of the Kconfig.
>
> But I wonder if Yes by default is good idea. Only very new CPUs will
> support this, right? Are they even available at the market? Should the
> help text say "if your CPU is Whatever Lake or newer, ...." :-) ?
>

CET enabled kernel runs on all x86-64 processors.  All my machines
are running the same CET enabled kernel binary.

-- 
H.J.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-18 21:06       ` H.J. Lu
@ 2020-09-18 21:17         ` Dave Hansen
  2020-09-18 21:22           ` H.J. Lu
  0 siblings, 1 reply; 39+ messages in thread
From: Dave Hansen @ 2020-09-18 21:17 UTC (permalink / raw)
  To: H.J. Lu, Pavel Machek
  Cc: Yu-cheng Yu, the arch/x86 maintainers, H. Peter Anvin,
	Thomas Gleixner, Ingo Molnar, LKML, open list:DOCUMENTATION,
	Linux-MM, linux-arch, Linux API, Arnd Bergmann, Andy Lutomirski,
	Balbir Singh, Borislav Petkov, Cyrill Gorcunov, Dave Hansen,
	Eugene Syromiatnikov, Florian Weimer, Jann Horn, Jonathan Corbet,
	Kees Cook, Mike Kravetz, Nadav Amit, Oleg Nesterov,
	Peter Zijlstra, Randy Dunlap, Ravi V. Shankar,
	Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

On 9/18/20 2:06 PM, H.J. Lu wrote:
> On Fri, Sep 18, 2020 at 2:00 PM Pavel Machek <pavel@ucw.cz> wrote:
>> On Fri 2020-09-18 12:32:57, Dave Hansen wrote:
>>> On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
>>>> Emulation of the legacy vsyscall page is required by some programs
>>>> built before 2013.  Newer programs after 2013 don't use it.
>>>> Disable vsyscall emulation when Control-flow Enforcement (CET) is
>>>> enabled to enhance security.
>>> How does this "enhance security"?
>>>
>>> What is the connection between vsyscall emulation and CET?
>> Boom.
>>
>> We don't break compatibility by default, and you should not tell
>> people to enable CET by default if you plan to do this.
>>
> Nothing will be broken.   CET enabled applications don't use/need
> vsyscall emulation.

Hi H.J.,

Could you explain your logic a bit more thoroughly, please?

I also suspect that Pavel was confused by your changelog where you said
that you do this when "CET is enabled".  Does enabled in this context mean:
1. Just CET support compiled in, or
2. Compiled in and on CET hardware, or
3. Compiled in to the kernel enabled in the app and running on CET
   hardware?

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-18 21:00     ` Pavel Machek
  2020-09-18 21:06       ` H.J. Lu
@ 2020-09-18 21:21       ` Yu, Yu-cheng
  2020-09-18 21:22         ` Pavel Machek
  1 sibling, 1 reply; 39+ messages in thread
From: Yu, Yu-cheng @ 2020-09-18 21:21 UTC (permalink / raw)
  To: Pavel Machek, Dave Hansen
  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, Peter Zijlstra, Randy Dunlap, Ravi V. Shankar,
	Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

On 9/18/2020 2:00 PM, Pavel Machek wrote:
> On Fri 2020-09-18 12:32:57, Dave Hansen wrote:
>> On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
>>> Emulation of the legacy vsyscall page is required by some programs
>>> built before 2013.  Newer programs after 2013 don't use it.
>>> Disable vsyscall emulation when Control-flow Enforcement (CET) is
>>> enabled to enhance security.
>>
>> How does this "enhance security"?
>>
>> What is the connection between vsyscall emulation and CET?
> 
> Boom.
> 
> We don't break compatibility by default, and you should not tell
> people to enable CET by default if you plan to do this.

I would revise the wording if there is another version.  What this patch 
does is:

If an application is compiled for CET and the system supports it, then 
the application cannot do vsyscall emulation.  Earlier we allow the 
emulation, and had a patch that fixes the shadow stack and endbr for the 
emulation code.  Since newer programs mostly do no do the emulation, we 
changed the patch do block it when attempted.

This patch would not block any legacy applications or any applications 
on older machines.

Yu-cheng

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-18 21:17         ` Dave Hansen
@ 2020-09-18 21:22           ` H.J. Lu
  2020-09-18 21:28             ` Dave Hansen
  0 siblings, 1 reply; 39+ messages in thread
From: H.J. Lu @ 2020-09-18 21:22 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Pavel Machek, Yu-cheng Yu, the arch/x86 maintainers,
	H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
	Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
	Florian Weimer, Jann Horn, Jonathan Corbet, Kees Cook,
	Mike Kravetz, Nadav Amit, Oleg Nesterov, Peter Zijlstra,
	Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin,
	Weijiang Yang

On Fri, Sep 18, 2020 at 2:17 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 9/18/20 2:06 PM, H.J. Lu wrote:
> > On Fri, Sep 18, 2020 at 2:00 PM Pavel Machek <pavel@ucw.cz> wrote:
> >> On Fri 2020-09-18 12:32:57, Dave Hansen wrote:
> >>> On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
> >>>> Emulation of the legacy vsyscall page is required by some programs
> >>>> built before 2013.  Newer programs after 2013 don't use it.
> >>>> Disable vsyscall emulation when Control-flow Enforcement (CET) is
> >>>> enabled to enhance security.
> >>> How does this "enhance security"?
> >>>
> >>> What is the connection between vsyscall emulation and CET?
> >> Boom.
> >>
> >> We don't break compatibility by default, and you should not tell
> >> people to enable CET by default if you plan to do this.
> >>
> > Nothing will be broken.   CET enabled applications don't use/need
> > vsyscall emulation.
>
> Hi H.J.,
>
> Could you explain your logic a bit more thoroughly, please?

Here is my CET slides for LPC 2020:

https://gitlab.com/cet-software/cet-smoke-test/-/wikis/uploads/09431a51248858e6f716a59065d732e2/CET-LPC-2020.pdf

which may have answers for most questions.

> I also suspect that Pavel was confused by your changelog where you said
> that you do this when "CET is enabled".  Does enabled in this context mean:
> 1. Just CET support compiled in, or
> 2. Compiled in and on CET hardware, or
> 3. Compiled in to the kernel enabled in the app and running on CET
>    hardware?

CET is enabled only in a process if

1. All components are CET enabled, and
2. CPU supports CET, and
3. Kernel supports CET.

-- 
H.J.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-18 21:21       ` Yu, Yu-cheng
@ 2020-09-18 21:22         ` Pavel Machek
  0 siblings, 0 replies; 39+ messages in thread
From: Pavel Machek @ 2020-09-18 21:22 UTC (permalink / raw)
  To: Yu, Yu-cheng
  Cc: Dave Hansen, 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, Peter Zijlstra,
	Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin,
	Weijiang Yang

[-- Attachment #1: Type: text/plain, Size: 1485 bytes --]

On Fri 2020-09-18 14:21:10, Yu, Yu-cheng wrote:
> On 9/18/2020 2:00 PM, Pavel Machek wrote:
> > On Fri 2020-09-18 12:32:57, Dave Hansen wrote:
> > > On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
> > > > Emulation of the legacy vsyscall page is required by some programs
> > > > built before 2013.  Newer programs after 2013 don't use it.
> > > > Disable vsyscall emulation when Control-flow Enforcement (CET) is
> > > > enabled to enhance security.
> > > 
> > > How does this "enhance security"?
> > > 
> > > What is the connection between vsyscall emulation and CET?
> > 
> > Boom.
> > 
> > We don't break compatibility by default, and you should not tell
> > people to enable CET by default if you plan to do this.
> 
> I would revise the wording if there is another version.  What this patch
> does is:
> 
> If an application is compiled for CET and the system supports it, then the
> application cannot do vsyscall emulation.  Earlier we allow the emulation,
> and had a patch that fixes the shadow stack and endbr for the emulation
> code.  Since newer programs mostly do no do the emulation, we changed the
> patch do block it when attempted.
> 
> This patch would not block any legacy applications or any applications on
> older machines.

Aha, makes sense, sorry for the noise.

Best regards,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-18 21:08       ` H.J. Lu
@ 2020-09-18 21:24         ` Pavel Machek
  2020-09-18 21:36           ` H.J. Lu
  0 siblings, 1 reply; 39+ messages in thread
From: Pavel Machek @ 2020-09-18 21:24 UTC (permalink / raw)
  To: H.J. Lu
  Cc: Randy Dunlap, Yu-cheng Yu, the arch/x86 maintainers,
	H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
	Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
	Florian Weimer, Jann Horn, Jonathan Corbet, Kees Cook,
	Mike Kravetz, Nadav Amit, Oleg Nesterov, Peter Zijlstra,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

[-- Attachment #1: Type: text/plain, Size: 1216 bytes --]

Hi!

> > > > +   help
> > > > +     Indirect Branch Tracking (IBT) provides protection against
> > > > +     CALL-/JMP-oriented programming attacks.  It is active when
> > > > +     the kernel has this feature enabled, and the processor and
> > > > +     the application support it.  When this feature is enabled,
> > > > +     legacy non-IBT applications continue to work, but without
> > > > +     IBT protection.
> > > > +
> > > > +     If unsure, say y
> > >
> > >         If unsure, say y.
> >
> > Actually, it would be "If unsure, say Y.", to be consistent with the
> > rest of the Kconfig.
> >
> > But I wonder if Yes by default is good idea. Only very new CPUs will
> > support this, right? Are they even available at the market? Should the
> > help text say "if your CPU is Whatever Lake or newer, ...." :-) ?
> >
> 
> CET enabled kernel runs on all x86-64 processors.  All my machines
> are running the same CET enabled kernel binary.

I believe that.

But enabling CET in kernel is useless on Core 2 Duo machine, right?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-18 20:59     ` Pavel Machek
  2020-09-18 21:08       ` H.J. Lu
@ 2020-09-18 21:25       ` Yu, Yu-cheng
  2020-09-18 21:40         ` Pavel Machek
  1 sibling, 1 reply; 39+ messages in thread
From: Yu, Yu-cheng @ 2020-09-18 21:25 UTC (permalink / raw)
  To: Pavel Machek, Randy Dunlap
  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, Peter Zijlstra, Ravi V. Shankar,
	Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

On 9/18/2020 1:59 PM, Pavel Machek wrote:
> On Fri 2020-09-18 13:24:13, Randy Dunlap wrote:
>> Hi,
>>
>> If you do another version of this:
>>
>> On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
>>> Introduce Kconfig option X86_INTEL_BRANCH_TRACKING_USER.
>>>
>>> Indirect Branch Tracking (IBT) provides protection against CALL-/JMP-
>>> oriented programming attacks.  It is active when the kernel has this
>>> feature enabled, and the processor and the application support it.
>>> When this feature is enabled, legacy non-IBT applications continue to
>>> work, but without IBT protection.
>>>
>>> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
>>> ---
>>> v10:
>>> - Change build-time CET check to config depends on.
>>>
>>>   arch/x86/Kconfig | 16 ++++++++++++++++
>>>   1 file changed, 16 insertions(+)
>>>
>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>>> index 6b6dad011763..b047e0a8d1c2 100644
>>> --- a/arch/x86/Kconfig
>>> +++ b/arch/x86/Kconfig
>>> @@ -1963,6 +1963,22 @@ config X86_INTEL_SHADOW_STACK_USER
>>>   
>>>   	  If unsure, say y.
>>>   
>>> +config X86_INTEL_BRANCH_TRACKING_USER
>>> +	prompt "Intel Indirect Branch Tracking for user-mode"
>>> +	def_bool n
>>> +	depends on CPU_SUP_INTEL && X86_64
>>> +	depends on $(cc-option,-fcf-protection)
>>> +	select X86_INTEL_CET
>>> +	help
>>> +	  Indirect Branch Tracking (IBT) provides protection against
>>> +	  CALL-/JMP-oriented programming attacks.  It is active when
>>> +	  the kernel has this feature enabled, and the processor and
>>> +	  the application support it.  When this feature is enabled,
>>> +	  legacy non-IBT applications continue to work, but without
>>> +	  IBT protection.
>>> +
>>> +	  If unsure, say y
>>
>> 	  If unsure, say y.
> 
> Actually, it would be "If unsure, say Y.", to be consistent with the
> rest of the Kconfig.
> 
> But I wonder if Yes by default is good idea. Only very new CPUs will
> support this, right? Are they even available at the market? Should the
> help text say "if your CPU is Whatever Lake or newer, ...." :-) ?

I will revise the wording if there is another version.  But a 
CET-capable kernel can run on legacy systems.  We have been testing that 
combination.

Yu-cheng

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-18 21:22           ` H.J. Lu
@ 2020-09-18 21:28             ` Dave Hansen
  0 siblings, 0 replies; 39+ messages in thread
From: Dave Hansen @ 2020-09-18 21:28 UTC (permalink / raw)
  To: H.J. Lu
  Cc: Pavel Machek, Yu-cheng Yu, the arch/x86 maintainers,
	H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
	Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
	Florian Weimer, Jann Horn, Jonathan Corbet, Kees Cook,
	Mike Kravetz, Nadav Amit, Oleg Nesterov, Peter Zijlstra,
	Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin,
	Weijiang Yang

> Here is my CET slides for LPC 2020:
> 
> https://gitlab.com/cet-software/cet-smoke-test/-/wikis/uploads/09431a51248858e6f716a59065d732e2/CET-LPC-2020.pdf
> 
> which may have answers for most questions.

Hi H.J.,

I know you're not super familiar with our kernel development process,
which might be why you've been repeatedly referring folks to your
presentations.  Our efforts here on the kernel mailing list are to
ensure that we have a nice standalone changelog in the kernel forever.
Great as your presentation might be, it isn't a part of the changelog.

I'm not asking these questions because I need them explained to me.  I'm
asking because I need Yu-cheng to ensure he has the answers in his
changelog when the patches are merged.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-18 21:24         ` Pavel Machek
@ 2020-09-18 21:36           ` H.J. Lu
  0 siblings, 0 replies; 39+ messages in thread
From: H.J. Lu @ 2020-09-18 21:36 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Randy Dunlap, Yu-cheng Yu, the arch/x86 maintainers,
	H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
	Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
	Florian Weimer, Jann Horn, Jonathan Corbet, Kees Cook,
	Mike Kravetz, Nadav Amit, Oleg Nesterov, Peter Zijlstra,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

On Fri, Sep 18, 2020 at 2:24 PM Pavel Machek <pavel@ucw.cz> wrote:
>
> Hi!
>
> > > > > +   help
> > > > > +     Indirect Branch Tracking (IBT) provides protection against
> > > > > +     CALL-/JMP-oriented programming attacks.  It is active when
> > > > > +     the kernel has this feature enabled, and the processor and
> > > > > +     the application support it.  When this feature is enabled,
> > > > > +     legacy non-IBT applications continue to work, but without
> > > > > +     IBT protection.
> > > > > +
> > > > > +     If unsure, say y
> > > >
> > > >         If unsure, say y.
> > >
> > > Actually, it would be "If unsure, say Y.", to be consistent with the
> > > rest of the Kconfig.
> > >
> > > But I wonder if Yes by default is good idea. Only very new CPUs will
> > > support this, right? Are they even available at the market? Should the
> > > help text say "if your CPU is Whatever Lake or newer, ...." :-) ?
> > >
> >
> > CET enabled kernel runs on all x86-64 processors.  All my machines
> > are running the same CET enabled kernel binary.
>
> I believe that.
>
> But enabling CET in kernel is useless on Core 2 Duo machine, right?
>

This is very important for CET kernel to run on Core 2 Duo machine.
Otherwise, a distro needs to provide 2 kernel binaries, one for CET
CPU and one for non-CET CPU.


-- 
H.J.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-18 21:25       ` Yu, Yu-cheng
@ 2020-09-18 21:40         ` Pavel Machek
  2020-09-18 21:46           ` H.J. Lu
  2020-09-21 22:30           ` Yu, Yu-cheng
  0 siblings, 2 replies; 39+ messages in thread
From: Pavel Machek @ 2020-09-18 21:40 UTC (permalink / raw)
  To: Yu, Yu-cheng
  Cc: Randy Dunlap, 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, Peter Zijlstra,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

[-- Attachment #1: Type: text/plain, Size: 2757 bytes --]

On Fri 2020-09-18 14:25:12, Yu, Yu-cheng wrote:
> On 9/18/2020 1:59 PM, Pavel Machek wrote:
> > On Fri 2020-09-18 13:24:13, Randy Dunlap wrote:
> > > Hi,
> > > 
> > > If you do another version of this:
> > > 
> > > On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
> > > > Introduce Kconfig option X86_INTEL_BRANCH_TRACKING_USER.
> > > > 
> > > > Indirect Branch Tracking (IBT) provides protection against CALL-/JMP-
> > > > oriented programming attacks.  It is active when the kernel has this
> > > > feature enabled, and the processor and the application support it.
> > > > When this feature is enabled, legacy non-IBT applications continue to
> > > > work, but without IBT protection.
> > > > 
> > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > ---
> > > > v10:
> > > > - Change build-time CET check to config depends on.
> > > > 
> > > >   arch/x86/Kconfig | 16 ++++++++++++++++
> > > >   1 file changed, 16 insertions(+)
> > > > 
> > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > > index 6b6dad011763..b047e0a8d1c2 100644
> > > > --- a/arch/x86/Kconfig
> > > > +++ b/arch/x86/Kconfig
> > > > @@ -1963,6 +1963,22 @@ config X86_INTEL_SHADOW_STACK_USER
> > > >   	  If unsure, say y.
> > > > +config X86_INTEL_BRANCH_TRACKING_USER
> > > > +	prompt "Intel Indirect Branch Tracking for user-mode"
> > > > +	def_bool n
> > > > +	depends on CPU_SUP_INTEL && X86_64
> > > > +	depends on $(cc-option,-fcf-protection)
> > > > +	select X86_INTEL_CET
> > > > +	help
> > > > +	  Indirect Branch Tracking (IBT) provides protection against
> > > > +	  CALL-/JMP-oriented programming attacks.  It is active when
> > > > +	  the kernel has this feature enabled, and the processor and
> > > > +	  the application support it.  When this feature is enabled,
> > > > +	  legacy non-IBT applications continue to work, but without
> > > > +	  IBT protection.
> > > > +
> > > > +	  If unsure, say y
> > > 
> > > 	  If unsure, say y.
> > 
> > Actually, it would be "If unsure, say Y.", to be consistent with the
> > rest of the Kconfig.
> > 
> > But I wonder if Yes by default is good idea. Only very new CPUs will
> > support this, right? Are they even available at the market? Should the
> > help text say "if your CPU is Whatever Lake or newer, ...." :-) ?
> 
> I will revise the wording if there is another version.  But a CET-capable
> kernel can run on legacy systems.  We have been testing that combination.

Yes, but enabling CET is unneccessary overhead on older systems. And
Kconfig is great place to explain that.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-18 21:40         ` Pavel Machek
@ 2020-09-18 21:46           ` H.J. Lu
  2020-09-18 22:03             ` Pavel Machek
  2020-09-21 22:30           ` Yu, Yu-cheng
  1 sibling, 1 reply; 39+ messages in thread
From: H.J. Lu @ 2020-09-18 21:46 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Yu, Yu-cheng, Randy Dunlap, the arch/x86 maintainers,
	H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
	Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
	Florian Weimer, Jann Horn, Jonathan Corbet, Kees Cook,
	Mike Kravetz, Nadav Amit, Oleg Nesterov, Peter Zijlstra,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

On Fri, Sep 18, 2020 at 2:40 PM Pavel Machek <pavel@ucw.cz> wrote:
>
> On Fri 2020-09-18 14:25:12, Yu, Yu-cheng wrote:
> > On 9/18/2020 1:59 PM, Pavel Machek wrote:
> > > On Fri 2020-09-18 13:24:13, Randy Dunlap wrote:
> > > > Hi,
> > > >
> > > > If you do another version of this:
> > > >
> > > > On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
> > > > > Introduce Kconfig option X86_INTEL_BRANCH_TRACKING_USER.
> > > > >
> > > > > Indirect Branch Tracking (IBT) provides protection against CALL-/JMP-
> > > > > oriented programming attacks.  It is active when the kernel has this
> > > > > feature enabled, and the processor and the application support it.
> > > > > When this feature is enabled, legacy non-IBT applications continue to
> > > > > work, but without IBT protection.
> > > > >
> > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > > ---
> > > > > v10:
> > > > > - Change build-time CET check to config depends on.
> > > > >
> > > > >   arch/x86/Kconfig | 16 ++++++++++++++++
> > > > >   1 file changed, 16 insertions(+)
> > > > >
> > > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > > > index 6b6dad011763..b047e0a8d1c2 100644
> > > > > --- a/arch/x86/Kconfig
> > > > > +++ b/arch/x86/Kconfig
> > > > > @@ -1963,6 +1963,22 @@ config X86_INTEL_SHADOW_STACK_USER
> > > > >           If unsure, say y.
> > > > > +config X86_INTEL_BRANCH_TRACKING_USER
> > > > > +       prompt "Intel Indirect Branch Tracking for user-mode"
> > > > > +       def_bool n
> > > > > +       depends on CPU_SUP_INTEL && X86_64
> > > > > +       depends on $(cc-option,-fcf-protection)
> > > > > +       select X86_INTEL_CET
> > > > > +       help
> > > > > +         Indirect Branch Tracking (IBT) provides protection against
> > > > > +         CALL-/JMP-oriented programming attacks.  It is active when
> > > > > +         the kernel has this feature enabled, and the processor and
> > > > > +         the application support it.  When this feature is enabled,
> > > > > +         legacy non-IBT applications continue to work, but without
> > > > > +         IBT protection.
> > > > > +
> > > > > +         If unsure, say y
> > > >
> > > >     If unsure, say y.
> > >
> > > Actually, it would be "If unsure, say Y.", to be consistent with the
> > > rest of the Kconfig.
> > >
> > > But I wonder if Yes by default is good idea. Only very new CPUs will
> > > support this, right? Are they even available at the market? Should the
> > > help text say "if your CPU is Whatever Lake or newer, ...." :-) ?
> >
> > I will revise the wording if there is another version.  But a CET-capable
> > kernel can run on legacy systems.  We have been testing that combination.
>
> Yes, but enabling CET is unneccessary overhead on older systems. And
> Kconfig is great place to explain that.
>

I can't tell any visible CET kernel overhead on my non-CET machines.

-- 
H.J.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-18 21:46           ` H.J. Lu
@ 2020-09-18 22:03             ` Pavel Machek
  0 siblings, 0 replies; 39+ messages in thread
From: Pavel Machek @ 2020-09-18 22:03 UTC (permalink / raw)
  To: H.J. Lu
  Cc: Yu, Yu-cheng, Randy Dunlap, the arch/x86 maintainers,
	H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
	Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
	Florian Weimer, Jann Horn, Jonathan Corbet, Kees Cook,
	Mike Kravetz, Nadav Amit, Oleg Nesterov, Peter Zijlstra,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

[-- Attachment #1: Type: text/plain, Size: 3525 bytes --]

On Fri 2020-09-18 14:46:12, H.J. Lu wrote:
> On Fri, Sep 18, 2020 at 2:40 PM Pavel Machek <pavel@ucw.cz> wrote:
> >
> > On Fri 2020-09-18 14:25:12, Yu, Yu-cheng wrote:
> > > On 9/18/2020 1:59 PM, Pavel Machek wrote:
> > > > On Fri 2020-09-18 13:24:13, Randy Dunlap wrote:
> > > > > Hi,
> > > > >
> > > > > If you do another version of this:
> > > > >
> > > > > On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
> > > > > > Introduce Kconfig option X86_INTEL_BRANCH_TRACKING_USER.
> > > > > >
> > > > > > Indirect Branch Tracking (IBT) provides protection against CALL-/JMP-
> > > > > > oriented programming attacks.  It is active when the kernel has this
> > > > > > feature enabled, and the processor and the application support it.
> > > > > > When this feature is enabled, legacy non-IBT applications continue to
> > > > > > work, but without IBT protection.
> > > > > >
> > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > > > ---
> > > > > > v10:
> > > > > > - Change build-time CET check to config depends on.
> > > > > >
> > > > > >   arch/x86/Kconfig | 16 ++++++++++++++++
> > > > > >   1 file changed, 16 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > > > > index 6b6dad011763..b047e0a8d1c2 100644
> > > > > > --- a/arch/x86/Kconfig
> > > > > > +++ b/arch/x86/Kconfig
> > > > > > @@ -1963,6 +1963,22 @@ config X86_INTEL_SHADOW_STACK_USER
> > > > > >           If unsure, say y.
> > > > > > +config X86_INTEL_BRANCH_TRACKING_USER
> > > > > > +       prompt "Intel Indirect Branch Tracking for user-mode"
> > > > > > +       def_bool n
> > > > > > +       depends on CPU_SUP_INTEL && X86_64
> > > > > > +       depends on $(cc-option,-fcf-protection)
> > > > > > +       select X86_INTEL_CET
> > > > > > +       help
> > > > > > +         Indirect Branch Tracking (IBT) provides protection against
> > > > > > +         CALL-/JMP-oriented programming attacks.  It is active when
> > > > > > +         the kernel has this feature enabled, and the processor and
> > > > > > +         the application support it.  When this feature is enabled,
> > > > > > +         legacy non-IBT applications continue to work, but without
> > > > > > +         IBT protection.
> > > > > > +
> > > > > > +         If unsure, say y
> > > > >
> > > > >     If unsure, say y.
> > > >
> > > > Actually, it would be "If unsure, say Y.", to be consistent with the
> > > > rest of the Kconfig.
> > > >
> > > > But I wonder if Yes by default is good idea. Only very new CPUs will
> > > > support this, right? Are they even available at the market? Should the
> > > > help text say "if your CPU is Whatever Lake or newer, ...." :-) ?
> > >
> > > I will revise the wording if there is another version.  But a CET-capable
> > > kernel can run on legacy systems.  We have been testing that combination.
> >
> > Yes, but enabling CET is unneccessary overhead on older systems. And
> > Kconfig is great place to explain that.
> >
> 
> I can't tell any visible CET kernel overhead on my non-CET machines.

I assume you are not a troll but you sound a bit like one.

Please list kernel size before and after enabling
X86_INTEL_CET option(s).

That's the overhead I'm talking about, and that's why Kconfig should
explain what machines this is useful on.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-18 19:23 ` [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled Yu-cheng Yu
  2020-09-18 19:32   ` Dave Hansen
@ 2020-09-19  0:11   ` Andy Lutomirski
  2020-09-21 16:22     ` Yu, Yu-cheng
  1 sibling, 1 reply; 39+ messages in thread
From: Andy Lutomirski @ 2020-09-19  0:11 UTC (permalink / raw)
  To: Yu-cheng Yu
  Cc: X86 ML, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, 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

On Fri, Sep 18, 2020 at 12:23 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> Emulation of the legacy vsyscall page is required by some programs
> built before 2013.  Newer programs after 2013 don't use it.
> Disable vsyscall emulation when Control-flow Enforcement (CET) is
> enabled to enhance security.
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
> v12:
> - Disable vsyscall emulation only when it is attempted (vs. at compile time).
>
>  arch/x86/entry/vsyscall/vsyscall_64.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
> index 44c33103a955..3196e963e365 100644
> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
> @@ -150,6 +150,15 @@ bool emulate_vsyscall(unsigned long error_code,
>
>         WARN_ON_ONCE(address != regs->ip);
>
> +#ifdef CONFIG_X86_INTEL_CET
> +       if (current->thread.cet.shstk_size ||
> +           current->thread.cet.ibt_enabled) {
> +               warn_bad_vsyscall(KERN_INFO, regs,
> +                                 "vsyscall attempted with cet enabled");
> +               return false;
> +       }

Nope, try again.  Having IBT on does *not* mean that every library in
the process knows that we have indirect branch tracking.  The legacy
bitmap exists for a reason.  Also, I want a way to flag programs as
not using the vsyscall page, but that flag should not be called CET.
And a process with vsyscalls off should not be able to read the
vsyscall page, and /proc/self/maps should be correct.

So you have some choices:

1. Drop this patch and make it work.

2. Add a real per-process vsyscall control.  Either make it depend on
vsyscall=xonly and wire it up correctly or actually make it work
correctly with vsyscall=emulate.

NAK to any hacks in this space.  Do it right or don't do it at all.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-19  0:11   ` Andy Lutomirski
@ 2020-09-21 16:22     ` Yu, Yu-cheng
  2020-09-21 22:37       ` Yu-cheng Yu
  0 siblings, 1 reply; 39+ messages in thread
From: Yu, Yu-cheng @ 2020-09-21 16:22 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: X86 ML, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, 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

On 9/18/2020 5:11 PM, Andy Lutomirski wrote:
> On Fri, Sep 18, 2020 at 12:23 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>>
>> Emulation of the legacy vsyscall page is required by some programs
>> built before 2013.  Newer programs after 2013 don't use it.
>> Disable vsyscall emulation when Control-flow Enforcement (CET) is
>> enabled to enhance security.
>>
>> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
>> ---
>> v12:
>> - Disable vsyscall emulation only when it is attempted (vs. at compile time).
>>
>>   arch/x86/entry/vsyscall/vsyscall_64.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
>> index 44c33103a955..3196e963e365 100644
>> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
>> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
>> @@ -150,6 +150,15 @@ bool emulate_vsyscall(unsigned long error_code,
>>
>>          WARN_ON_ONCE(address != regs->ip);
>>
>> +#ifdef CONFIG_X86_INTEL_CET
>> +       if (current->thread.cet.shstk_size ||
>> +           current->thread.cet.ibt_enabled) {
>> +               warn_bad_vsyscall(KERN_INFO, regs,
>> +                                 "vsyscall attempted with cet enabled");
>> +               return false;
>> +       }
> 
> Nope, try again.  Having IBT on does *not* mean that every library in
> the process knows that we have indirect branch tracking.  The legacy
> bitmap exists for a reason.  Also, I want a way to flag programs as
> not using the vsyscall page, but that flag should not be called CET.
> And a process with vsyscalls off should not be able to read the
> vsyscall page, and /proc/self/maps should be correct.
> 
> So you have some choices:
> 
> 1. Drop this patch and make it work.
> 
> 2. Add a real per-process vsyscall control.  Either make it depend on
> vsyscall=xonly and wire it up correctly or actually make it work
> correctly with vsyscall=emulate.
> 
> NAK to any hacks in this space.  Do it right or don't do it at all.
> 

We can drop this patch, and bring back the previous patch that fixes up 
shadow stack and ibt.  That makes vsyscall emulation work correctly, and 
does not force the application to do anything different from what is 
working now.  I will post the previous patch as a reply to this thread 
so that people can make comments on it.

Yu-cheng

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-18 21:40         ` Pavel Machek
  2020-09-18 21:46           ` H.J. Lu
@ 2020-09-21 22:30           ` Yu, Yu-cheng
  2020-09-21 22:41             ` Dave Hansen
  1 sibling, 1 reply; 39+ messages in thread
From: Yu, Yu-cheng @ 2020-09-21 22:30 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Randy Dunlap, 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, Peter Zijlstra,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

On 9/18/2020 2:40 PM, Pavel Machek wrote:
> On Fri 2020-09-18 14:25:12, Yu, Yu-cheng wrote:
>> On 9/18/2020 1:59 PM, Pavel Machek wrote:
>>> On Fri 2020-09-18 13:24:13, Randy Dunlap wrote:
>>>> Hi,
>>>>
>>>> If you do another version of this:
>>>>
>>>> On 9/18/20 12:23 PM, Yu-cheng Yu wrote:
>>>>> Introduce Kconfig option X86_INTEL_BRANCH_TRACKING_USER.
>>>>>
>>>>> Indirect Branch Tracking (IBT) provides protection against CALL-/JMP-
>>>>> oriented programming attacks.  It is active when the kernel has this
>>>>> feature enabled, and the processor and the application support it.
>>>>> When this feature is enabled, legacy non-IBT applications continue to
>>>>> work, but without IBT protection.
>>>>>
>>>>> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
>>>>> ---
>>>>> v10:
>>>>> - Change build-time CET check to config depends on.
>>>>>
>>>>>    arch/x86/Kconfig | 16 ++++++++++++++++
>>>>>    1 file changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>>>>> index 6b6dad011763..b047e0a8d1c2 100644
>>>>> --- a/arch/x86/Kconfig
>>>>> +++ b/arch/x86/Kconfig
>>>>> @@ -1963,6 +1963,22 @@ config X86_INTEL_SHADOW_STACK_USER
>>>>>    	  If unsure, say y.
>>>>> +config X86_INTEL_BRANCH_TRACKING_USER
>>>>> +	prompt "Intel Indirect Branch Tracking for user-mode"
>>>>> +	def_bool n
>>>>> +	depends on CPU_SUP_INTEL && X86_64
>>>>> +	depends on $(cc-option,-fcf-protection)
>>>>> +	select X86_INTEL_CET
>>>>> +	help
>>>>> +	  Indirect Branch Tracking (IBT) provides protection against
>>>>> +	  CALL-/JMP-oriented programming attacks.  It is active when
>>>>> +	  the kernel has this feature enabled, and the processor and
>>>>> +	  the application support it.  When this feature is enabled,
>>>>> +	  legacy non-IBT applications continue to work, but without
>>>>> +	  IBT protection.
>>>>> +
>>>>> +	  If unsure, say y
>>>>
>>>> 	  If unsure, say y.
>>>
>>> Actually, it would be "If unsure, say Y.", to be consistent with the
>>> rest of the Kconfig.
>>>
>>> But I wonder if Yes by default is good idea. Only very new CPUs will
>>> support this, right? Are they even available at the market? Should the
>>> help text say "if your CPU is Whatever Lake or newer, ...." :-) ?
>>
>> I will revise the wording if there is another version.  But a CET-capable
>> kernel can run on legacy systems.  We have been testing that combination.
> 
> Yes, but enabling CET is unneccessary overhead on older systems. And
> Kconfig is great place to explain that.

Maybe I'll add:

If the kernel is to be used only on older systems that do not support 
IBT, and the size of the binary is important, you can save 900 KB by 
disabling this feature.

Otherwise, if unsure, say Y.

How is that?

Thanks,
Yu-cheng

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-21 16:22     ` Yu, Yu-cheng
@ 2020-09-21 22:37       ` Yu-cheng Yu
  2020-09-21 23:48         ` Andy Lutomirski
  0 siblings, 1 reply; 39+ messages in thread
From: Yu-cheng Yu @ 2020-09-21 22:37 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: X86 ML, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, 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

On Mon, 2020-09-21 at 09:22 -0700, Yu, Yu-cheng wrote:
> On 9/18/2020 5:11 PM, Andy Lutomirski wrote:
> > On Fri, Sep 18, 2020 at 12:23 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > Emulation of the legacy vsyscall page is required by some programs
> > > built before 2013.  Newer programs after 2013 don't use it.
> > > Disable vsyscall emulation when Control-flow Enforcement (CET) is
> > > enabled to enhance security.
> > > 
> > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
[...]
> > 
> > Nope, try again.  Having IBT on does *not* mean that every library in
> > the process knows that we have indirect branch tracking.  The legacy
> > bitmap exists for a reason.  Also, I want a way to flag programs as
> > not using the vsyscall page, but that flag should not be called CET.
> > And a process with vsyscalls off should not be able to read the
> > vsyscall page, and /proc/self/maps should be correct.
> > 
> > So you have some choices:
> > 
> > 1. Drop this patch and make it work.
> > 
> > 2. Add a real per-process vsyscall control.  Either make it depend on
> > vsyscall=xonly and wire it up correctly or actually make it work
> > correctly with vsyscall=emulate.
> > 
> > NAK to any hacks in this space.  Do it right or don't do it at all.
> > 
> 
> We can drop this patch, and bring back the previous patch that fixes up 
> shadow stack and ibt.  That makes vsyscall emulation work correctly, and 
> does not force the application to do anything different from what is 
> working now.  I will post the previous patch as a reply to this thread 
> so that people can make comments on it.
> 
> Yu-cheng

Here is the patch:

------

From dfdee39c795ee5dcee2c77f6ba344a61f4d8124b Mon Sep 17 00:00:00 2001
From: Yu-cheng Yu <yu-cheng.yu@intel.com>
Date: Thu, 29 Nov 2018 14:15:38 -0800
Subject: [PATCH 34/43] x86/vsyscall/64: Fixup Shadow Stack and Indirect Branch
 Tracking for vsyscall emulation

Vsyscall entry points are effectively branch targets.  Mark them with
ENDBR64 opcodes.  When emulating the RET instruction, unwind the shadow
stack and reset IBT state machine.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/entry/vsyscall/vsyscall_64.c     | 29 +++++++++++++++++++++++
 arch/x86/entry/vsyscall/vsyscall_emu_64.S |  9 +++++++
 arch/x86/entry/vsyscall/vsyscall_trace.h  |  1 +
 3 files changed, 39 insertions(+)

diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c
b/arch/x86/entry/vsyscall/vsyscall_64.c
index 44c33103a955..0131c9f7f9c5 100644
--- a/arch/x86/entry/vsyscall/vsyscall_64.c
+++ b/arch/x86/entry/vsyscall/vsyscall_64.c
@@ -38,6 +38,9 @@
 #include <asm/fixmap.h>
 #include <asm/traps.h>
 #include <asm/paravirt.h>
+#include <asm/fpu/xstate.h>
+#include <asm/fpu/types.h>
+#include <asm/fpu/internal.h>
 
 #define CREATE_TRACE_POINTS
 #include "vsyscall_trace.h"
@@ -286,6 +289,32 @@ bool emulate_vsyscall(unsigned long error_code,
 	/* Emulate a ret instruction. */
 	regs->ip = caller;
 	regs->sp += 8;
+
+	if (current->thread.cet.shstk_size ||
+	    current->thread.cet.ibt_enabled) {
+		u64 r;
+
+		fpregs_lock();
+		if (test_thread_flag(TIF_NEED_FPU_LOAD))
+			__fpregs_load_activate();
+
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+		/* Fixup branch tracking */
+		if (current->thread.cet.ibt_enabled) {
+			rdmsrl(MSR_IA32_U_CET, r);
+			wrmsrl(MSR_IA32_U_CET, r & ~CET_WAIT_ENDBR);
+		}
+#endif
+
+#ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
+		/* Unwind shadow stack. */
+		if (current->thread.cet.shstk_size) {
+			rdmsrl(MSR_IA32_PL3_SSP, r);
+			wrmsrl(MSR_IA32_PL3_SSP, r + 8);
+		}
+#endif
+		fpregs_unlock();
+	}
 	return true;
 
 sigsegv:
diff --git a/arch/x86/entry/vsyscall/vsyscall_emu_64.S
b/arch/x86/entry/vsyscall/vsyscall_emu_64.S
index 2e203f3a25a7..040696333457 100644
--- a/arch/x86/entry/vsyscall/vsyscall_emu_64.S
+++ b/arch/x86/entry/vsyscall/vsyscall_emu_64.S
@@ -17,16 +17,25 @@ __PAGE_ALIGNED_DATA
 	.type __vsyscall_page, @object
 __vsyscall_page:
 
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+	endbr64
+#endif
 	mov $__NR_gettimeofday, %rax
 	syscall
 	ret
 
 	.balign 1024, 0xcc
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+	endbr64
+#endif
 	mov $__NR_time, %rax
 	syscall
 	ret
 
 	.balign 1024, 0xcc
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+	endbr64
+#endif
 	mov $__NR_getcpu, %rax
 	syscall
 	ret
diff --git a/arch/x86/entry/vsyscall/vsyscall_trace.h
b/arch/x86/entry/vsyscall/vsyscall_trace.h
index 3c3f9765a85c..7aa2101ada44 100644
--- a/arch/x86/entry/vsyscall/vsyscall_trace.h
+++ b/arch/x86/entry/vsyscall/vsyscall_trace.h
@@ -25,6 +25,7 @@ TRACE_EVENT(emulate_vsyscall,
 #endif
 
 #undef TRACE_INCLUDE_PATH
+#undef TRACE_INCLUDE_FILE
 #define TRACE_INCLUDE_PATH ../../arch/x86/entry/vsyscall/
 #define TRACE_INCLUDE_FILE vsyscall_trace
 #include <trace/define_trace.h>
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-21 22:30           ` Yu, Yu-cheng
@ 2020-09-21 22:41             ` Dave Hansen
  2020-09-21 22:47               ` Yu, Yu-cheng
  2020-09-21 22:52               ` Pavel Machek
  0 siblings, 2 replies; 39+ messages in thread
From: Dave Hansen @ 2020-09-21 22:41 UTC (permalink / raw)
  To: Yu, Yu-cheng, Pavel Machek
  Cc: Randy Dunlap, 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, Peter Zijlstra,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

On 9/21/20 3:30 PM, Yu, Yu-cheng wrote:
> +config X86_INTEL_BRANCH_TRACKING_USER
> +    prompt "Intel Indirect Branch Tracking for user-mode" 

Take the "Intel " and "INTEL_" out, please.  It will only cause us all
pain later if some of our x86 compatriots decide to implement this.

> If the kernel is to be used only on older systems that do not support
> IBT, and the size of the binary is important, you can save 900 KB by
> disabling this feature.
> 
> Otherwise, if unsure, say Y.

900k seems like a *lot*.  Where the heck does that come from?

Also, comments like that don't age very well.  Consider:

	Support for this feature is only known to be present on Intel
	processors released in 2020 or later.  This feature is also
	known to increase kernel text size substantially.

	If unsure, say N.

The 900KB is probably wrong today in a lot of configurations, and will;
only get *more* wrong over time.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-21 22:41             ` Dave Hansen
@ 2020-09-21 22:47               ` Yu, Yu-cheng
       [not found]                 ` <9cf234db-d0f7-0466-be2c-afe04eb76759@intel.com>
  2020-09-21 22:52               ` Pavel Machek
  1 sibling, 1 reply; 39+ messages in thread
From: Yu, Yu-cheng @ 2020-09-21 22:47 UTC (permalink / raw)
  To: Dave Hansen, Pavel Machek
  Cc: Randy Dunlap, 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, Peter Zijlstra,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

On 9/21/2020 3:41 PM, Dave Hansen wrote:
> On 9/21/20 3:30 PM, Yu, Yu-cheng wrote:
>> +config X86_INTEL_BRANCH_TRACKING_USER
>> +    prompt "Intel Indirect Branch Tracking for user-mode"
> 
> Take the "Intel " and "INTEL_" out, please.  It will only cause us all
> pain later if some of our x86 compatriots decide to implement this.
> 
>> If the kernel is to be used only on older systems that do not support
>> IBT, and the size of the binary is important, you can save 900 KB by
>> disabling this feature.
>>
>> Otherwise, if unsure, say Y.
> 
> 900k seems like a *lot*.  Where the heck does that come from?
> 
> Also, comments like that don't age very well.  Consider:
> 
> 	Support for this feature is only known to be present on Intel
> 	processors released in 2020 or later.  This feature is also
> 	known to increase kernel text size substantially.
> 
> 	If unsure, say N.
> 

Thanks!

> The 900KB is probably wrong today in a lot of configurations, and will;
> only get *more* wrong over time.
> 

I was talking about the vmlinux file, and probably should have said 
bzImage size, which has 14 KB increase when CET is enabled.

Yu-cheng

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  2020-09-21 22:41             ` Dave Hansen
  2020-09-21 22:47               ` Yu, Yu-cheng
@ 2020-09-21 22:52               ` Pavel Machek
  1 sibling, 0 replies; 39+ messages in thread
From: Pavel Machek @ 2020-09-21 22:52 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Yu, Yu-cheng, Randy Dunlap, 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, Peter Zijlstra, Ravi V. Shankar,
	Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

[-- Attachment #1: Type: text/plain, Size: 1085 bytes --]

Hi!

> > +config X86_INTEL_BRANCH_TRACKING_USER
> > +    prompt "Intel Indirect Branch Tracking for user-mode" 
> 
> Take the "Intel " and "INTEL_" out, please.  It will only cause us all
> pain later if some of our x86 compatriots decide to implement this.

Are other x86 manufacturers legally allowed to implement that?

> > If the kernel is to be used only on older systems that do not support
> > IBT, and the size of the binary is important, you can save 900 KB by
> > disabling this feature.
> > 
> > Otherwise, if unsure, say Y.
> 
> 900k seems like a *lot*.  Where the heck does that come from?
> 
> Also, comments like that don't age very well.  Consider:
> 
> 	Support for this feature is only known to be present on Intel
> 	processors released in 2020 or later.  This feature is also
> 	known to increase kernel text size substantially.
> 
> 	If unsure, say N.

That is much better, thanks.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
       [not found]                 ` <9cf234db-d0f7-0466-be2c-afe04eb76759@intel.com>
@ 2020-09-21 23:27                   ` Yu, Yu-cheng
  0 siblings, 0 replies; 39+ messages in thread
From: Yu, Yu-cheng @ 2020-09-21 23:27 UTC (permalink / raw)
  To: Dave Hansen, Pavel Machek
  Cc: Randy Dunlap, 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, Peter Zijlstra,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin, Weijiang Yang

On 9/21/2020 3:54 PM, Dave Hansen wrote:
> On 9/21/20 3:47 PM, Yu, Yu-cheng wrote:
>>> The 900KB is probably wrong today in a lot of configurations, and will;
>>> only get *more* wrong over time.
>>
>> I was talking about the vmlinux file, and probably should have said
>> bzImage size, which has 14 KB increase when CET is enabled.
> 
> Well, vmlinux size is important too.  1 page of vmlinux size means one
> fewer page of memory available for real use.
> 
> I would really encourage you when you write to try to be specific and
> use as much plain language as possible without being verbose.  Most
> people understand things like "this feature increases kernel text size".
>   I wouldn't expect most folks who can type "make oldconfig; make
> install" to understands the difference between vmlinux and bzImage.
> 

Ok, thanks!

Yu-cheng

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-21 22:37       ` Yu-cheng Yu
@ 2020-09-21 23:48         ` Andy Lutomirski
  2020-09-23 21:29           ` Sean Christopherson
       [not found]           ` <b3defc91-1e8e-d0d5-2ac3-3861a7e3355c@intel.com>
  0 siblings, 2 replies; 39+ messages in thread
From: Andy Lutomirski @ 2020-09-21 23:48 UTC (permalink / raw)
  To: Yu-cheng Yu
  Cc: Andy Lutomirski, X86 ML, H. Peter Anvin, Thomas Gleixner,
	Ingo Molnar, LKML, open list:DOCUMENTATION, Linux-MM, linux-arch,
	Linux API, Arnd Bergmann, 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

On Mon, Sep 21, 2020 at 3:37 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> On Mon, 2020-09-21 at 09:22 -0700, Yu, Yu-cheng wrote:
> > On 9/18/2020 5:11 PM, Andy Lutomirski wrote:
> > > On Fri, Sep 18, 2020 at 12:23 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > > Emulation of the legacy vsyscall page is required by some programs
> > > > built before 2013.  Newer programs after 2013 don't use it.
> > > > Disable vsyscall emulation when Control-flow Enforcement (CET) is
> > > > enabled to enhance security.
> > > >
> > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> [...]
> > >
> > > Nope, try again.  Having IBT on does *not* mean that every library in
> > > the process knows that we have indirect branch tracking.  The legacy
> > > bitmap exists for a reason.  Also, I want a way to flag programs as
> > > not using the vsyscall page, but that flag should not be called CET.
> > > And a process with vsyscalls off should not be able to read the
> > > vsyscall page, and /proc/self/maps should be correct.
> > >
> > > So you have some choices:
> > >
> > > 1. Drop this patch and make it work.
> > >
> > > 2. Add a real per-process vsyscall control.  Either make it depend on
> > > vsyscall=xonly and wire it up correctly or actually make it work
> > > correctly with vsyscall=emulate.
> > >
> > > NAK to any hacks in this space.  Do it right or don't do it at all.
> > >
> >
> > We can drop this patch, and bring back the previous patch that fixes up
> > shadow stack and ibt.  That makes vsyscall emulation work correctly, and
> > does not force the application to do anything different from what is
> > working now.  I will post the previous patch as a reply to this thread
> > so that people can make comments on it.
> >
> > Yu-cheng
>
> Here is the patch:
>
> ------
>
> From dfdee39c795ee5dcee2c77f6ba344a61f4d8124b Mon Sep 17 00:00:00 2001
> From: Yu-cheng Yu <yu-cheng.yu@intel.com>
> Date: Thu, 29 Nov 2018 14:15:38 -0800
> Subject: [PATCH 34/43] x86/vsyscall/64: Fixup Shadow Stack and Indirect Branch
>  Tracking for vsyscall emulation
>
> Vsyscall entry points are effectively branch targets.  Mark them with
> ENDBR64 opcodes.  When emulating the RET instruction, unwind the shadow
> stack and reset IBT state machine.
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
>  arch/x86/entry/vsyscall/vsyscall_64.c     | 29 +++++++++++++++++++++++
>  arch/x86/entry/vsyscall/vsyscall_emu_64.S |  9 +++++++
>  arch/x86/entry/vsyscall/vsyscall_trace.h  |  1 +
>  3 files changed, 39 insertions(+)
>
> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c
> b/arch/x86/entry/vsyscall/vsyscall_64.c
> index 44c33103a955..0131c9f7f9c5 100644
> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
> @@ -38,6 +38,9 @@
>  #include <asm/fixmap.h>
>  #include <asm/traps.h>
>  #include <asm/paravirt.h>
> +#include <asm/fpu/xstate.h>
> +#include <asm/fpu/types.h>
> +#include <asm/fpu/internal.h>
>
>  #define CREATE_TRACE_POINTS
>  #include "vsyscall_trace.h"
> @@ -286,6 +289,32 @@ bool emulate_vsyscall(unsigned long error_code,
>         /* Emulate a ret instruction. */
>         regs->ip = caller;
>         regs->sp += 8;
> +
> +       if (current->thread.cet.shstk_size ||
> +           current->thread.cet.ibt_enabled) {
> +               u64 r;
> +
> +               fpregs_lock();
> +               if (test_thread_flag(TIF_NEED_FPU_LOAD))
> +                       __fpregs_load_activate();

Wouldn't this be nicer if you operated on the memory image, not the registers?

> +
> +#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
> +               /* Fixup branch tracking */
> +               if (current->thread.cet.ibt_enabled) {
> +                       rdmsrl(MSR_IA32_U_CET, r);
> +                       wrmsrl(MSR_IA32_U_CET, r & ~CET_WAIT_ENDBR);
> +               }
> +#endif

Seems reasonable on first glance.

> +
> +#ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
> +               /* Unwind shadow stack. */
> +               if (current->thread.cet.shstk_size) {
> +                       rdmsrl(MSR_IA32_PL3_SSP, r);
> +                       wrmsrl(MSR_IA32_PL3_SSP, r + 8);
> +               }
> +#endif

What happens if the result is noncanonical?  A quick skim of the SDM
didn't find anything.  This latter issue goes away if you operate on
the memory image, though -- writing a bogus value is just fine, since
the FP restore will handle it.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-21 23:48         ` Andy Lutomirski
@ 2020-09-23 21:29           ` Sean Christopherson
       [not found]             ` <a2e872ef-5539-c7c1-49ca-95d590f3b92a@intel.com>
       [not found]           ` <b3defc91-1e8e-d0d5-2ac3-3861a7e3355c@intel.com>
  1 sibling, 1 reply; 39+ messages in thread
From: Sean Christopherson @ 2020-09-23 21:29 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Yu-cheng Yu, X86 ML, H. Peter Anvin, Thomas Gleixner,
	Ingo Molnar, LKML, open list:DOCUMENTATION, Linux-MM, linux-arch,
	Linux API, Arnd Bergmann, 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

On Mon, Sep 21, 2020 at 04:48:25PM -0700, Andy Lutomirski wrote:
> On Mon, Sep 21, 2020 at 3:37 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c
> > b/arch/x86/entry/vsyscall/vsyscall_64.c
> > index 44c33103a955..0131c9f7f9c5 100644
> > --- a/arch/x86/entry/vsyscall/vsyscall_64.c
> > +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
> > @@ -38,6 +38,9 @@
> >  #include <asm/fixmap.h>
> >  #include <asm/traps.h>
> >  #include <asm/paravirt.h>
> > +#include <asm/fpu/xstate.h>
> > +#include <asm/fpu/types.h>
> > +#include <asm/fpu/internal.h>
> >
> >  #define CREATE_TRACE_POINTS
> >  #include "vsyscall_trace.h"
> > @@ -286,6 +289,32 @@ bool emulate_vsyscall(unsigned long error_code,
> >         /* Emulate a ret instruction. */
> >         regs->ip = caller;
> >         regs->sp += 8;
> > +
> > +       if (current->thread.cet.shstk_size ||
> > +           current->thread.cet.ibt_enabled) {
> > +               u64 r;
> > +
> > +               fpregs_lock();
> > +               if (test_thread_flag(TIF_NEED_FPU_LOAD))
> > +                       __fpregs_load_activate();
> 
> Wouldn't this be nicer if you operated on the memory image, not the registers?
> 
> > +
> > +#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
> > +               /* Fixup branch tracking */
> > +               if (current->thread.cet.ibt_enabled) {
> > +                       rdmsrl(MSR_IA32_U_CET, r);
> > +                       wrmsrl(MSR_IA32_U_CET, r & ~CET_WAIT_ENDBR);
> > +               }
> > +#endif
> 
> Seems reasonable on first glance.
> 
> > +
> > +#ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
> > +               /* Unwind shadow stack. */
> > +               if (current->thread.cet.shstk_size) {
> > +                       rdmsrl(MSR_IA32_PL3_SSP, r);
> > +                       wrmsrl(MSR_IA32_PL3_SSP, r + 8);
> > +               }
> > +#endif
> 
> What happens if the result is noncanonical?  A quick skim of the SDM
> didn't find anything.  This latter issue goes away if you operate on
> the memory image, though -- writing a bogus value is just fine, since
> the FP restore will handle it.

#GP, the SSP MSRs do canonical checks.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
       [not found]           ` <b3defc91-1e8e-d0d5-2ac3-3861a7e3355c@intel.com>
@ 2020-09-23 21:34             ` Andy Lutomirski
  2020-09-23 22:07               ` Yu, Yu-cheng
  0 siblings, 1 reply; 39+ messages in thread
From: Andy Lutomirski @ 2020-09-23 21:34 UTC (permalink / raw)
  To: Yu, Yu-cheng
  Cc: Andy Lutomirski, X86 ML, H. Peter Anvin, Thomas Gleixner,
	Ingo Molnar, LKML, open list:DOCUMENTATION, Linux-MM, linux-arch,
	Linux API, Arnd Bergmann, 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

On Tue, Sep 22, 2020 at 10:46 AM Yu, Yu-cheng <yu-cheng.yu@intel.com> wrote:
>
> On 9/21/2020 4:48 PM, Andy Lutomirski wrote:
> > On Mon, Sep 21, 2020 at 3:37 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> >>
> >> On Mon, 2020-09-21 at 09:22 -0700, Yu, Yu-cheng wrote:
>
> [...]
>
> >>
> >> Here is the patch:
> >>
> >> ------
> >>
> >>  From dfdee39c795ee5dcee2c77f6ba344a61f4d8124b Mon Sep 17 00:00:00 2001
> >> From: Yu-cheng Yu <yu-cheng.yu@intel.com>
> >> Date: Thu, 29 Nov 2018 14:15:38 -0800
> >> Subject: [PATCH 34/43] x86/vsyscall/64: Fixup Shadow Stack and Indirect Branch
> >>   Tracking for vsyscall emulation
> >>
> >> Vsyscall entry points are effectively branch targets.  Mark them with
> >> ENDBR64 opcodes.  When emulating the RET instruction, unwind the shadow
> >> stack and reset IBT state machine.
> >>
> >> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> >> ---
> >>   arch/x86/entry/vsyscall/vsyscall_64.c     | 29 +++++++++++++++++++++++
> >>   arch/x86/entry/vsyscall/vsyscall_emu_64.S |  9 +++++++
> >>   arch/x86/entry/vsyscall/vsyscall_trace.h  |  1 +
> >>   3 files changed, 39 insertions(+)
> >>
> >> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c
> >> b/arch/x86/entry/vsyscall/vsyscall_64.c
> >> index 44c33103a955..0131c9f7f9c5 100644
> >> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
> >> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
> >> @@ -38,6 +38,9 @@
> >>   #include <asm/fixmap.h>
> >>   #include <asm/traps.h>
> >>   #include <asm/paravirt.h>
> >> +#include <asm/fpu/xstate.h>
> >> +#include <asm/fpu/types.h>
> >> +#include <asm/fpu/internal.h>
> >>
> >>   #define CREATE_TRACE_POINTS
> >>   #include "vsyscall_trace.h"
> >> @@ -286,6 +289,32 @@ bool emulate_vsyscall(unsigned long error_code,
> >>          /* Emulate a ret instruction. */
> >>          regs->ip = caller;
> >>          regs->sp += 8;
> >> +
> >> +       if (current->thread.cet.shstk_size ||
> >> +           current->thread.cet.ibt_enabled) {
> >> +               u64 r;
> >> +
> >> +               fpregs_lock();
> >> +               if (test_thread_flag(TIF_NEED_FPU_LOAD))
> >> +                       __fpregs_load_activate();
> >
> > Wouldn't this be nicer if you operated on the memory image, not the registers?
>
> Do you mean writing to the XSAVES area?

Yes.

>
> >
> >> +
> >> +#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
> >> +               /* Fixup branch tracking */
> >> +               if (current->thread.cet.ibt_enabled) {
> >> +                       rdmsrl(MSR_IA32_U_CET, r);
> >> +                       wrmsrl(MSR_IA32_U_CET, r & ~CET_WAIT_ENDBR);
> >> +               }
> >> +#endif
> >
> > Seems reasonable on first glance.
> >
> >> +
> >> +#ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
> >> +               /* Unwind shadow stack. */
> >> +               if (current->thread.cet.shstk_size) {
> >> +                       rdmsrl(MSR_IA32_PL3_SSP, r);
> >> +                       wrmsrl(MSR_IA32_PL3_SSP, r + 8);
> >> +               }
> >> +#endif
> >
> > What happens if the result is noncanonical?  A quick skim of the SDM
> > didn't find anything.  This latter issue goes away if you operate on
> > the memory image, though -- writing a bogus value is just fine, since
> > the FP restore will handle it.
> >
>
> At this point, the MSR's value can still be valid or is already saved to
> memory.  If we are going to write to memory, then the MSR must be saved
> first.  So I chose to do __fpregs_load_activate() and write the MSR.
>
> Maybe we can check the address before writing it to the MSR?

Performance is almost irrelevant here, and the writing-to-XSAVES-area
approach should have the benefit that the exception handling and
signaling happens for free.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-23 21:34             ` Andy Lutomirski
@ 2020-09-23 22:07               ` Yu, Yu-cheng
  0 siblings, 0 replies; 39+ messages in thread
From: Yu, Yu-cheng @ 2020-09-23 22:07 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: X86 ML, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, 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

On 9/23/2020 2:34 PM, Andy Lutomirski wrote:
> On Tue, Sep 22, 2020 at 10:46 AM Yu, Yu-cheng <yu-cheng.yu@intel.com> wrote:
>>
>> On 9/21/2020 4:48 PM, Andy Lutomirski wrote:
>>> On Mon, Sep 21, 2020 at 3:37 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>>>>
>>>> On Mon, 2020-09-21 at 09:22 -0700, Yu, Yu-cheng wrote:
>>
>> [...]
>>
>>>>
>>>> Here is the patch:
>>>>
>>>> ------
>>>>
>>>>   From dfdee39c795ee5dcee2c77f6ba344a61f4d8124b Mon Sep 17 00:00:00 2001
>>>> From: Yu-cheng Yu <yu-cheng.yu@intel.com>
>>>> Date: Thu, 29 Nov 2018 14:15:38 -0800
>>>> Subject: [PATCH 34/43] x86/vsyscall/64: Fixup Shadow Stack and Indirect Branch
>>>>    Tracking for vsyscall emulation
>>>>
>>>> Vsyscall entry points are effectively branch targets.  Mark them with
>>>> ENDBR64 opcodes.  When emulating the RET instruction, unwind the shadow
>>>> stack and reset IBT state machine.
>>>>
>>>> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
>>>> ---
>>>>    arch/x86/entry/vsyscall/vsyscall_64.c     | 29 +++++++++++++++++++++++
>>>>    arch/x86/entry/vsyscall/vsyscall_emu_64.S |  9 +++++++
>>>>    arch/x86/entry/vsyscall/vsyscall_trace.h  |  1 +
>>>>    3 files changed, 39 insertions(+)
>>>>
>>>> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c
>>>> b/arch/x86/entry/vsyscall/vsyscall_64.c
>>>> index 44c33103a955..0131c9f7f9c5 100644
>>>> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
>>>> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
>>>> @@ -38,6 +38,9 @@
>>>>    #include <asm/fixmap.h>
>>>>    #include <asm/traps.h>
>>>>    #include <asm/paravirt.h>
>>>> +#include <asm/fpu/xstate.h>
>>>> +#include <asm/fpu/types.h>
>>>> +#include <asm/fpu/internal.h>
>>>>
>>>>    #define CREATE_TRACE_POINTS
>>>>    #include "vsyscall_trace.h"
>>>> @@ -286,6 +289,32 @@ bool emulate_vsyscall(unsigned long error_code,
>>>>           /* Emulate a ret instruction. */
>>>>           regs->ip = caller;
>>>>           regs->sp += 8;
>>>> +
>>>> +       if (current->thread.cet.shstk_size ||
>>>> +           current->thread.cet.ibt_enabled) {
>>>> +               u64 r;
>>>> +
>>>> +               fpregs_lock();
>>>> +               if (test_thread_flag(TIF_NEED_FPU_LOAD))
>>>> +                       __fpregs_load_activate();
>>>
>>> Wouldn't this be nicer if you operated on the memory image, not the registers?
>>
>> Do you mean writing to the XSAVES area?
> 
> Yes.
> 
>>
>>>
>>>> +
>>>> +#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
>>>> +               /* Fixup branch tracking */
>>>> +               if (current->thread.cet.ibt_enabled) {
>>>> +                       rdmsrl(MSR_IA32_U_CET, r);
>>>> +                       wrmsrl(MSR_IA32_U_CET, r & ~CET_WAIT_ENDBR);
>>>> +               }
>>>> +#endif
>>>
>>> Seems reasonable on first glance.
>>>
>>>> +
>>>> +#ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
>>>> +               /* Unwind shadow stack. */
>>>> +               if (current->thread.cet.shstk_size) {
>>>> +                       rdmsrl(MSR_IA32_PL3_SSP, r);
>>>> +                       wrmsrl(MSR_IA32_PL3_SSP, r + 8);
>>>> +               }
>>>> +#endif
>>>
>>> What happens if the result is noncanonical?  A quick skim of the SDM
>>> didn't find anything.  This latter issue goes away if you operate on
>>> the memory image, though -- writing a bogus value is just fine, since
>>> the FP restore will handle it.
>>>
>>
>> At this point, the MSR's value can still be valid or is already saved to
>> memory.  If we are going to write to memory, then the MSR must be saved
>> first.  So I chose to do __fpregs_load_activate() and write the MSR.
>>
>> Maybe we can check the address before writing it to the MSR?
> 
> Performance is almost irrelevant here, and the writing-to-XSAVES-area
> approach should have the benefit that the exception handling and
> signaling happens for free.
> 

Ok, I will change it.

Thanks,
Yu-cheng

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
       [not found]               ` <e7c20f4c-23a0-4a34-3895-c4f60993ec41@intel.com>
@ 2020-09-23 22:20                 ` Yu, Yu-cheng
  2020-09-23 22:47                   ` Andy Lutomirski
  0 siblings, 1 reply; 39+ messages in thread
From: Yu, Yu-cheng @ 2020-09-23 22:20 UTC (permalink / raw)
  To: Dave Hansen, Sean Christopherson, Andy Lutomirski
  Cc: X86 ML, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, 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

On 9/23/2020 3:08 PM, Dave Hansen wrote:
> On 9/23/20 3:06 PM, Yu, Yu-cheng wrote:
>> I think I'll add a check here for (r + 8) >= TASK_SIZE_MAX.  It is
>> better than getting a fault.
> 
> There's also wrmsr_safe().
> 
Yes, thanks.

Since I am going to change this to:

fpu__prepare_write(), then write to the XSAVES area.

The kernel does not expect XRSTORS to fail ("Bad FPU state detected..." 
message).  So maybe still check the address first.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-23 22:20                 ` Yu, Yu-cheng
@ 2020-09-23 22:47                   ` Andy Lutomirski
  2020-09-23 22:53                     ` Dave Hansen
  0 siblings, 1 reply; 39+ messages in thread
From: Andy Lutomirski @ 2020-09-23 22:47 UTC (permalink / raw)
  To: Yu, Yu-cheng
  Cc: Dave Hansen, Sean Christopherson, Andy Lutomirski, X86 ML,
	H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
	open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, 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

On Wed, Sep 23, 2020 at 3:20 PM Yu, Yu-cheng <yu-cheng.yu@intel.com> wrote:
>
> On 9/23/2020 3:08 PM, Dave Hansen wrote:
> > On 9/23/20 3:06 PM, Yu, Yu-cheng wrote:
> >> I think I'll add a check here for (r + 8) >= TASK_SIZE_MAX.  It is
> >> better than getting a fault.
> >
> > There's also wrmsr_safe().
> >
> Yes, thanks.
>
> Since I am going to change this to:
>
> fpu__prepare_write(), then write to the XSAVES area.
>
> The kernel does not expect XRSTORS to fail ("Bad FPU state detected..."
> message).  So maybe still check the address first.

Surely there are plenty of ways to use ptrace() to poke garbage into
the FPU state.  We should be able to handle this type of failure
somewhat gracefully.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled
  2020-09-23 22:47                   ` Andy Lutomirski
@ 2020-09-23 22:53                     ` Dave Hansen
  0 siblings, 0 replies; 39+ messages in thread
From: Dave Hansen @ 2020-09-23 22:53 UTC (permalink / raw)
  To: Andy Lutomirski, Yu, Yu-cheng
  Cc: Sean Christopherson, X86 ML, H. Peter Anvin, Thomas Gleixner,
	Ingo Molnar, LKML, open list:DOCUMENTATION, Linux-MM, linux-arch,
	Linux API, Arnd Bergmann, 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

On 9/23/20 3:47 PM, Andy Lutomirski wrote:
> On Wed, Sep 23, 2020 at 3:20 PM Yu, Yu-cheng <yu-cheng.yu@intel.com> wrote:
>> On 9/23/2020 3:08 PM, Dave Hansen wrote:
>>> On 9/23/20 3:06 PM, Yu, Yu-cheng wrote:
>>>> I think I'll add a check here for (r + 8) >= TASK_SIZE_MAX.  It is
>>>> better than getting a fault.
>>> There's also wrmsr_safe().
>>>
>> Yes, thanks.
>>
>> Since I am going to change this to:
>>
>> fpu__prepare_write(), then write to the XSAVES area.
>>
>> The kernel does not expect XRSTORS to fail ("Bad FPU state detected..."
>> message).  So maybe still check the address first.
> Surely there are plenty of ways to use ptrace() to poke garbage into
> the FPU state.  We should be able to handle this type of failure
> somewhat gracefully.

Yeah, agreed.  I'd much rather make XRSTORS able to #GP gracefully than
teach the kernel exhaustively about every possible error condition it
can encounter.

We *might* want to do something like to preserve the warning if the task
hasn't been ptrace'd, or had the memory buffer written to directly or
tainted in another way.

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2020-09-23 22:53 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-18 19:23 [PATCH v12 0/8] Control-flow Enforcement: Indirect Branch Tracking Yu-cheng Yu
2020-09-18 19:23 ` [PATCH v12 1/8] x86/cet/ibt: Add Kconfig option for user-mode " Yu-cheng Yu
2020-09-18 20:24   ` Randy Dunlap
2020-09-18 20:59     ` Pavel Machek
2020-09-18 21:08       ` H.J. Lu
2020-09-18 21:24         ` Pavel Machek
2020-09-18 21:36           ` H.J. Lu
2020-09-18 21:25       ` Yu, Yu-cheng
2020-09-18 21:40         ` Pavel Machek
2020-09-18 21:46           ` H.J. Lu
2020-09-18 22:03             ` Pavel Machek
2020-09-21 22:30           ` Yu, Yu-cheng
2020-09-21 22:41             ` Dave Hansen
2020-09-21 22:47               ` Yu, Yu-cheng
     [not found]                 ` <9cf234db-d0f7-0466-be2c-afe04eb76759@intel.com>
2020-09-21 23:27                   ` Yu, Yu-cheng
2020-09-21 22:52               ` Pavel Machek
2020-09-18 19:23 ` [PATCH v12 4/8] x86/cet/ibt: ELF header parsing for " Yu-cheng Yu
2020-09-18 19:23 ` [PATCH v12 5/8] x86/cet/ibt: Update arch_prctl functions " Yu-cheng Yu
2020-09-18 19:23 ` [PATCH v12 6/8] x86/vdso/32: Add ENDBR32 to __kernel_vsyscall entry point Yu-cheng Yu
2020-09-18 19:23 ` [PATCH v12 7/8] x86/vdso: Insert endbr32/endbr64 to vDSO Yu-cheng Yu
2020-09-18 19:23 ` [PATCH v12 8/8] x86: Disallow vsyscall emulation when CET is enabled Yu-cheng Yu
2020-09-18 19:32   ` Dave Hansen
2020-09-18 21:00     ` Pavel Machek
2020-09-18 21:06       ` H.J. Lu
2020-09-18 21:17         ` Dave Hansen
2020-09-18 21:22           ` H.J. Lu
2020-09-18 21:28             ` Dave Hansen
2020-09-18 21:21       ` Yu, Yu-cheng
2020-09-18 21:22         ` Pavel Machek
2020-09-19  0:11   ` Andy Lutomirski
2020-09-21 16:22     ` Yu, Yu-cheng
2020-09-21 22:37       ` Yu-cheng Yu
2020-09-21 23:48         ` Andy Lutomirski
2020-09-23 21:29           ` Sean Christopherson
     [not found]             ` <a2e872ef-5539-c7c1-49ca-95d590f3b92a@intel.com>
     [not found]               ` <e7c20f4c-23a0-4a34-3895-c4f60993ec41@intel.com>
2020-09-23 22:20                 ` Yu, Yu-cheng
2020-09-23 22:47                   ` Andy Lutomirski
2020-09-23 22:53                     ` Dave Hansen
     [not found]           ` <b3defc91-1e8e-d0d5-2ac3-3861a7e3355c@intel.com>
2020-09-23 21:34             ` Andy Lutomirski
2020-09-23 22:07               ` Yu, Yu-cheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).