[RFC PATCH v4 0/9] Control Flow Enforcement: Branch Tracking, PTRACE

[RFC PATCH v4 0/9] Control Flow Enforcement: Branch Tracking, PTRACE

[Yu-cheng Yu]

The previous version of CET patches can be found in the following
link:

  https://lkml.org/lkml/2018/8/30/582

Summary of changes from v3:

  Move IBT legacy code bitmap allocation back to when the application
  requests it.  Most application do not need the bitmap.  It is only
  used when an application does dlopen() a legacy library.

  In the previous version, we pre-allocated the bitmap for every IBT-
  enabled application to avoid creating a hole in the linear address.
  However, this created a problem when the system has limited memory.

H.J. Lu (1):
  x86: Insert endbr32/endbr64 to vDSO

Yu-cheng Yu (8):
  x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  x86/cet/ibt: User-mode indirect branch tracking support
  x86/cet/ibt: Add IBT legacy code bitmap allocation function
  mm/mmap: Add IBT bitmap size to address space limit check
  x86/cet/ibt: ELF header parsing for IBT
  x86/cet/ibt: Add arch_prctl functions for IBT
  x86/cet/ibt: Add ENDBR to op-code-map
  x86/cet: Add PTRACE interface for CET

 arch/x86/Kconfig                              | 12 +++
 arch/x86/Makefile                             |  7 ++
 arch/x86/entry/vdso/.gitignore                |  4 +
 arch/x86/entry/vdso/Makefile                  | 12 ++-
 arch/x86/entry/vdso/vdso-layout.lds.S         |  1 +
 arch/x86/include/asm/cet.h                    |  8 ++
 arch/x86/include/asm/disabled-features.h      |  8 +-
 arch/x86/include/asm/fpu/regset.h             |  7 +-
 arch/x86/include/uapi/asm/elf_property.h      |  1 +
 arch/x86/include/uapi/asm/prctl.h             |  1 +
 arch/x86/include/uapi/asm/resource.h          |  5 ++
 arch/x86/kernel/cet.c                         | 76 +++++++++++++++++++
 arch/x86/kernel/cet_prctl.c                   | 38 +++++++++-
 arch/x86/kernel/cpu/common.c                  | 20 ++++-
 arch/x86/kernel/elf.c                         |  8 +-
 arch/x86/kernel/fpu/regset.c                  | 41 ++++++++++
 arch/x86/kernel/process.c                     |  2 +
 arch/x86/kernel/ptrace.c                      | 16 ++++
 arch/x86/lib/x86-opcode-map.txt               | 13 +++-
 include/uapi/asm-generic/resource.h           |  3 +
 include/uapi/linux/elf.h                      |  1 +
 mm/mmap.c                                     | 12 ++-
 tools/objtool/arch/x86/lib/x86-opcode-map.txt | 13 +++-
 23 files changed, 296 insertions(+), 13 deletions(-)

-- 
2.17.1

[RFC PATCH v4 0/9] Control Flow Enforcement: Branch Tracking, PTRACE

[Yu-cheng Yu]

The previous version of CET patches can be found in the following
link:

  https://lkml.org/lkml/2018/8/30/582

Summary of changes from v3:

  Move IBT legacy code bitmap allocation back to when the application
  requests it.  Most application do not need the bitmap.  It is only
  used when an application does dlopen() a legacy library.

  In the previous version, we pre-allocated the bitmap for every IBT-
  enabled application to avoid creating a hole in the linear address.
  However, this created a problem when the system has limited memory.

H.J. Lu (1):
  x86: Insert endbr32/endbr64 to vDSO

Yu-cheng Yu (8):
  x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
  x86/cet/ibt: User-mode indirect branch tracking support
  x86/cet/ibt: Add IBT legacy code bitmap allocation function
  mm/mmap: Add IBT bitmap size to address space limit check
  x86/cet/ibt: ELF header parsing for IBT
  x86/cet/ibt: Add arch_prctl functions for IBT
  x86/cet/ibt: Add ENDBR to op-code-map
  x86/cet: Add PTRACE interface for CET

 arch/x86/Kconfig                              | 12 +++
 arch/x86/Makefile                             |  7 ++
 arch/x86/entry/vdso/.gitignore                |  4 +
 arch/x86/entry/vdso/Makefile                  | 12 ++-
 arch/x86/entry/vdso/vdso-layout.lds.S         |  1 +
 arch/x86/include/asm/cet.h                    |  8 ++
 arch/x86/include/asm/disabled-features.h      |  8 +-
 arch/x86/include/asm/fpu/regset.h             |  7 +-
 arch/x86/include/uapi/asm/elf_property.h      |  1 +
 arch/x86/include/uapi/asm/prctl.h             |  1 +
 arch/x86/include/uapi/asm/resource.h          |  5 ++
 arch/x86/kernel/cet.c                         | 76 +++++++++++++++++++
 arch/x86/kernel/cet_prctl.c                   | 38 +++++++++-
 arch/x86/kernel/cpu/common.c                  | 20 ++++-
 arch/x86/kernel/elf.c                         |  8 +-
 arch/x86/kernel/fpu/regset.c                  | 41 ++++++++++
 arch/x86/kernel/process.c                     |  2 +
 arch/x86/kernel/ptrace.c                      | 16 ++++
 arch/x86/lib/x86-opcode-map.txt               | 13 +++-
 include/uapi/asm-generic/resource.h           |  3 +
 include/uapi/linux/elf.h                      |  1 +
 mm/mmap.c                                     | 12 ++-
 tools/objtool/arch/x86/lib/x86-opcode-map.txt | 13 +++-
 23 files changed, 296 insertions(+), 13 deletions(-)

-- 
2.17.1

[RFC PATCH v4 1/9] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking

[Yu-cheng Yu]

The user-mode indirect branch tracking support is done mostly by
GCC to insert ENDBR64/ENDBR32 instructions at branch targets.
The kernel provides CPUID enumeration, feature MSR setup and
the allocation of legacy bitmap.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/Kconfig  | 12 ++++++++++++
 arch/x86/Makefile |  7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 6377125543cc..2a0ff538a229 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1941,6 +1941,18 @@ 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
+	select X86_INTEL_CET
+	select ARCH_HAS_PROGRAM_PROPERTIES
+	---help---
+	  Indirect Branch Tracking provides hardware protection against return-/jmp-
+	  oriented programming attacks.
+
+	  If unsure, say y
+
 config EFI
 	bool "EFI runtime service support"
 	depends on ACPI
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index b28842b80295..ff652bba849f 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -159,6 +159,13 @@ ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
   endif
 endif
 
+# Check compiler ibt support
+ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+  ifeq ($(call cc-option-yn, -fcf-protection=branch), n)
+      $(error CONFIG_X86_INTEL_BRANCH_TRACKING_USER not supported by compiler)
+  endif
+endif
+
 #
 # If the function graph tracer is used with mcount instead of fentry,
 # '-maccumulate-outgoing-args' is needed to prevent a GCC bug
-- 
2.17.1

[RFC PATCH v4 1/9] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking

[Yu-cheng Yu]

The user-mode indirect branch tracking support is done mostly by
GCC to insert ENDBR64/ENDBR32 instructions at branch targets.
The kernel provides CPUID enumeration, feature MSR setup and
the allocation of legacy bitmap.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/Kconfig  | 12 ++++++++++++
 arch/x86/Makefile |  7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 6377125543cc..2a0ff538a229 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1941,6 +1941,18 @@ 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
+	select X86_INTEL_CET
+	select ARCH_HAS_PROGRAM_PROPERTIES
+	---help---
+	  Indirect Branch Tracking provides hardware protection against return-/jmp-
+	  oriented programming attacks.
+
+	  If unsure, say y
+
 config EFI
 	bool "EFI runtime service support"
 	depends on ACPI
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index b28842b80295..ff652bba849f 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -159,6 +159,13 @@ ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
   endif
 endif
 
+# Check compiler ibt support
+ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+  ifeq ($(call cc-option-yn, -fcf-protection=branch), n)
+      $(error CONFIG_X86_INTEL_BRANCH_TRACKING_USER not supported by compiler)
+  endif
+endif
+
 #
 # If the function graph tracer is used with mcount instead of fentry,
 # '-maccumulate-outgoing-args' is needed to prevent a GCC bug
-- 
2.17.1

[RFC PATCH v4 2/9] x86/cet/ibt: User-mode indirect branch tracking support

[Yu-cheng Yu]

Add user-mode indirect branch tracking enabling/disabling
and supporting routines.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/include/asm/cet.h               |  8 ++++++
 arch/x86/include/asm/disabled-features.h |  8 +++++-
 arch/x86/kernel/cet.c                    | 31 ++++++++++++++++++++++++
 arch/x86/kernel/cpu/common.c             | 20 ++++++++++++++-
 arch/x86/kernel/process.c                |  1 +
 5 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cet.h b/arch/x86/include/asm/cet.h
index 212bd68e31d3..1fea93fd436a 100644
--- a/arch/x86/include/asm/cet.h
+++ b/arch/x86/include/asm/cet.h
@@ -12,8 +12,11 @@ struct task_struct;
 struct cet_status {
 	unsigned long	shstk_base;
 	unsigned long	shstk_size;
+	unsigned long	ibt_bitmap_addr;
+	unsigned long	ibt_bitmap_size;
 	unsigned int	locked:1;
 	unsigned int	shstk_enabled:1;
+	unsigned int	ibt_enabled:1;
 };
 
 #ifdef CONFIG_X86_INTEL_CET
@@ -25,6 +28,9 @@ void cet_disable_shstk(void);
 void cet_disable_free_shstk(struct task_struct *p);
 int cet_restore_signal(unsigned long ssp);
 int cet_setup_signal(bool ia32, unsigned long rstor, unsigned long *new_ssp);
+int cet_setup_ibt(void);
+int cet_setup_ibt_bitmap(void);
+void cet_disable_ibt(void);
 #else
 static inline int prctl_cet(int option, unsigned long arg2) { return 0; }
 static inline int cet_setup_shstk(void) { return 0; }
@@ -35,6 +41,8 @@ static inline void cet_disable_free_shstk(struct task_struct *p) {}
 static inline int cet_restore_signal(unsigned long ssp) { return 0; }
 static inline int cet_setup_signal(bool ia32, unsigned long rstor,
 				   unsigned long *new_ssp) { return 0; }
+static inline int cet_setup_ibt(void) { return 0; }
+static inline void cet_disable_ibt(void) {}
 #endif
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index 3624a11e5ba6..ce5bdaf0f1ff 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -62,6 +62,12 @@
 #define DISABLE_SHSTK	(1<<(X86_FEATURE_SHSTK & 31))
 #endif
 
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+#define DISABLE_IBT	0
+#else
+#define DISABLE_IBT	(1<<(X86_FEATURE_IBT & 31))
+#endif
+
 /*
  * Make sure to add features to the correct mask
  */
@@ -72,7 +78,7 @@
 #define DISABLED_MASK4	(DISABLE_PCID)
 #define DISABLED_MASK5	0
 #define DISABLED_MASK6	0
-#define DISABLED_MASK7	(DISABLE_PTI)
+#define DISABLED_MASK7	(DISABLE_PTI|DISABLE_IBT)
 #define DISABLED_MASK8	0
 #define DISABLED_MASK9	(DISABLE_MPX)
 #define DISABLED_MASK10	0
diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
index 1c2689738604..6adfe795d692 100644
--- a/arch/x86/kernel/cet.c
+++ b/arch/x86/kernel/cet.c
@@ -12,6 +12,8 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/sched/signal.h>
+#include <linux/vmalloc.h>
+#include <linux/bitops.h>
 #include <asm/msr.h>
 #include <asm/user.h>
 #include <asm/fpu/xstate.h>
@@ -283,3 +285,32 @@ int cet_setup_signal(bool ia32, unsigned long rstor_addr,
 	set_shstk_ptr(ssp);
 	return 0;
 }
+
+int cet_setup_ibt(void)
+{
+	u64 r;
+
+	if (!cpu_feature_enabled(X86_FEATURE_IBT))
+		return -EOPNOTSUPP;
+
+	rdmsrl(MSR_IA32_U_CET, r);
+	r |= (MSR_IA32_CET_ENDBR_EN | MSR_IA32_CET_NO_TRACK_EN);
+	wrmsrl(MSR_IA32_U_CET, r);
+
+	current->thread.cet.ibt_enabled = 1;
+	return 0;
+}
+
+void cet_disable_ibt(void)
+{
+	u64 r;
+
+	if (!cpu_feature_enabled(X86_FEATURE_IBT))
+		return;
+
+	rdmsrl(MSR_IA32_U_CET, r);
+	r &= ~(MSR_IA32_CET_ENDBR_EN | MSR_IA32_CET_LEG_IW_EN |
+	       MSR_IA32_CET_NO_TRACK_EN);
+	wrmsrl(MSR_IA32_U_CET, r);
+	current->thread.cet.ibt_enabled = 0;
+}
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index bffa9ef47832..230f65ee881e 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -413,7 +413,8 @@ __setup("nopku", setup_disable_pku);
 
 static __always_inline void setup_cet(struct cpuinfo_x86 *c)
 {
-	if (cpu_feature_enabled(X86_FEATURE_SHSTK))
+	if (cpu_feature_enabled(X86_FEATURE_SHSTK) ||
+	    cpu_feature_enabled(X86_FEATURE_IBT))
 		cr4_set_bits(X86_CR4_CET);
 }
 
@@ -434,6 +435,23 @@ static __init int setup_disable_shstk(char *s)
 __setup("no_cet_shstk", setup_disable_shstk);
 #endif
 
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+static __init int setup_disable_ibt(char *s)
+{
+	/* require an exact match without trailing characters */
+	if (strlen(s))
+		return 0;
+
+	if (!boot_cpu_has(X86_FEATURE_IBT))
+		return 1;
+
+	setup_clear_cpu_cap(X86_FEATURE_IBT);
+	pr_info("x86: 'no_cet_ibt' specified, disabling Branch Tracking\n");
+	return 1;
+}
+__setup("no_cet_ibt", setup_disable_ibt);
+#endif
+
 /*
  * Some CPU features depend on higher CPUID levels, which may not always
  * be available due to CPUID level capping or broken virtualization
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 251b8714f9a3..ac0ea9c7e89f 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -137,6 +137,7 @@ void flush_thread(void)
 	memset(tsk->thread.tls_array, 0, sizeof(tsk->thread.tls_array));
 
 	cet_disable_shstk();
+	cet_disable_ibt();
 	fpu__clear(&tsk->thread.fpu);
 }
 
-- 
2.17.1

[RFC PATCH v4 2/9] x86/cet/ibt: User-mode indirect branch tracking support

[Yu-cheng Yu]

Add user-mode indirect branch tracking enabling/disabling
and supporting routines.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/include/asm/cet.h               |  8 ++++++
 arch/x86/include/asm/disabled-features.h |  8 +++++-
 arch/x86/kernel/cet.c                    | 31 ++++++++++++++++++++++++
 arch/x86/kernel/cpu/common.c             | 20 ++++++++++++++-
 arch/x86/kernel/process.c                |  1 +
 5 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cet.h b/arch/x86/include/asm/cet.h
index 212bd68e31d3..1fea93fd436a 100644
--- a/arch/x86/include/asm/cet.h
+++ b/arch/x86/include/asm/cet.h
@@ -12,8 +12,11 @@ struct task_struct;
 struct cet_status {
 	unsigned long	shstk_base;
 	unsigned long	shstk_size;
+	unsigned long	ibt_bitmap_addr;
+	unsigned long	ibt_bitmap_size;
 	unsigned int	locked:1;
 	unsigned int	shstk_enabled:1;
+	unsigned int	ibt_enabled:1;
 };
 
 #ifdef CONFIG_X86_INTEL_CET
@@ -25,6 +28,9 @@ void cet_disable_shstk(void);
 void cet_disable_free_shstk(struct task_struct *p);
 int cet_restore_signal(unsigned long ssp);
 int cet_setup_signal(bool ia32, unsigned long rstor, unsigned long *new_ssp);
+int cet_setup_ibt(void);
+int cet_setup_ibt_bitmap(void);
+void cet_disable_ibt(void);
 #else
 static inline int prctl_cet(int option, unsigned long arg2) { return 0; }
 static inline int cet_setup_shstk(void) { return 0; }
@@ -35,6 +41,8 @@ static inline void cet_disable_free_shstk(struct task_struct *p) {}
 static inline int cet_restore_signal(unsigned long ssp) { return 0; }
 static inline int cet_setup_signal(bool ia32, unsigned long rstor,
 				   unsigned long *new_ssp) { return 0; }
+static inline int cet_setup_ibt(void) { return 0; }
+static inline void cet_disable_ibt(void) {}
 #endif
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index 3624a11e5ba6..ce5bdaf0f1ff 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -62,6 +62,12 @@
 #define DISABLE_SHSTK	(1<<(X86_FEATURE_SHSTK & 31))
 #endif
 
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+#define DISABLE_IBT	0
+#else
+#define DISABLE_IBT	(1<<(X86_FEATURE_IBT & 31))
+#endif
+
 /*
  * Make sure to add features to the correct mask
  */
@@ -72,7 +78,7 @@
 #define DISABLED_MASK4	(DISABLE_PCID)
 #define DISABLED_MASK5	0
 #define DISABLED_MASK6	0
-#define DISABLED_MASK7	(DISABLE_PTI)
+#define DISABLED_MASK7	(DISABLE_PTI|DISABLE_IBT)
 #define DISABLED_MASK8	0
 #define DISABLED_MASK9	(DISABLE_MPX)
 #define DISABLED_MASK10	0
diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
index 1c2689738604..6adfe795d692 100644
--- a/arch/x86/kernel/cet.c
+++ b/arch/x86/kernel/cet.c
@@ -12,6 +12,8 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/sched/signal.h>
+#include <linux/vmalloc.h>
+#include <linux/bitops.h>
 #include <asm/msr.h>
 #include <asm/user.h>
 #include <asm/fpu/xstate.h>
@@ -283,3 +285,32 @@ int cet_setup_signal(bool ia32, unsigned long rstor_addr,
 	set_shstk_ptr(ssp);
 	return 0;
 }
+
+int cet_setup_ibt(void)
+{
+	u64 r;
+
+	if (!cpu_feature_enabled(X86_FEATURE_IBT))
+		return -EOPNOTSUPP;
+
+	rdmsrl(MSR_IA32_U_CET, r);
+	r |= (MSR_IA32_CET_ENDBR_EN | MSR_IA32_CET_NO_TRACK_EN);
+	wrmsrl(MSR_IA32_U_CET, r);
+
+	current->thread.cet.ibt_enabled = 1;
+	return 0;
+}
+
+void cet_disable_ibt(void)
+{
+	u64 r;
+
+	if (!cpu_feature_enabled(X86_FEATURE_IBT))
+		return;
+
+	rdmsrl(MSR_IA32_U_CET, r);
+	r &= ~(MSR_IA32_CET_ENDBR_EN | MSR_IA32_CET_LEG_IW_EN |
+	       MSR_IA32_CET_NO_TRACK_EN);
+	wrmsrl(MSR_IA32_U_CET, r);
+	current->thread.cet.ibt_enabled = 0;
+}
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index bffa9ef47832..230f65ee881e 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -413,7 +413,8 @@ __setup("nopku", setup_disable_pku);
 
 static __always_inline void setup_cet(struct cpuinfo_x86 *c)
 {
-	if (cpu_feature_enabled(X86_FEATURE_SHSTK))
+	if (cpu_feature_enabled(X86_FEATURE_SHSTK) ||
+	    cpu_feature_enabled(X86_FEATURE_IBT))
 		cr4_set_bits(X86_CR4_CET);
 }
 
@@ -434,6 +435,23 @@ static __init int setup_disable_shstk(char *s)
 __setup("no_cet_shstk", setup_disable_shstk);
 #endif
 
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+static __init int setup_disable_ibt(char *s)
+{
+	/* require an exact match without trailing characters */
+	if (strlen(s))
+		return 0;
+
+	if (!boot_cpu_has(X86_FEATURE_IBT))
+		return 1;
+
+	setup_clear_cpu_cap(X86_FEATURE_IBT);
+	pr_info("x86: 'no_cet_ibt' specified, disabling Branch Tracking\n");
+	return 1;
+}
+__setup("no_cet_ibt", setup_disable_ibt);
+#endif
+
 /*
  * Some CPU features depend on higher CPUID levels, which may not always
  * be available due to CPUID level capping or broken virtualization
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 251b8714f9a3..ac0ea9c7e89f 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -137,6 +137,7 @@ void flush_thread(void)
 	memset(tsk->thread.tls_array, 0, sizeof(tsk->thread.tls_array));
 
 	cet_disable_shstk();
+	cet_disable_ibt();
 	fpu__clear(&tsk->thread.fpu);
 }
 
-- 
2.17.1

Re: [RFC PATCH v4 2/9] x86/cet/ibt: User-mode indirect branch tracking support

[Eugene Syromiatnikov]

On Fri, Sep 21, 2018 at 08:05:46AM -0700, Yu-cheng Yu wrote:

> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index bffa9ef47832..230f65ee881e 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c

> @@ -434,6 +435,23 @@ static __init int setup_disable_shstk(char *s)
>  __setup("no_cet_shstk", setup_disable_shstk);
>  #endif
>  
> +#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
> +static __init int setup_disable_ibt(char *s)
> +{
> +	/* require an exact match without trailing characters */
> +	if (strlen(s))
> +		return 0;

"s[0] (!= '\0')" will do the same check in constant time.

Re: [RFC PATCH v4 2/9] x86/cet/ibt: User-mode indirect branch tracking support

[Eugene Syromiatnikov]

On Fri, Sep 21, 2018 at 08:05:46AM -0700, Yu-cheng Yu wrote:

> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index bffa9ef47832..230f65ee881e 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c

> @@ -434,6 +435,23 @@ static __init int setup_disable_shstk(char *s)
>  __setup("no_cet_shstk", setup_disable_shstk);
>  #endif
>  
> +#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
> +static __init int setup_disable_ibt(char *s)
> +{
> +	/* require an exact match without trailing characters */
> +	if (strlen(s))
> +		return 0;

"s[0] (!= '\0')" will do the same check in constant time.

[RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Yu-cheng Yu]

Indirect branch tracking provides an optional legacy code bitmap
that indicates locations of non-IBT compatible code.  When set,
each bit in the bitmap represents a page in the linear address is
legacy code.

We allocate the bitmap only when the application requests it.
Most applications do not need the bitmap.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
index 6adfe795d692..a65d9745af08 100644
--- a/arch/x86/kernel/cet.c
+++ b/arch/x86/kernel/cet.c
@@ -314,3 +314,48 @@ void cet_disable_ibt(void)
 	wrmsrl(MSR_IA32_U_CET, r);
 	current->thread.cet.ibt_enabled = 0;
 }
+
+int cet_setup_ibt_bitmap(void)
+{
+	u64 r;
+	unsigned long bitmap;
+	unsigned long size;
+
+	if (!cpu_feature_enabled(X86_FEATURE_IBT))
+		return -EOPNOTSUPP;
+
+	if (!current->thread.cet.ibt_bitmap_addr) {
+		/*
+		 * Calculate size and put in thread header.
+		 * may_expand_vm() needs this information.
+		 */
+		size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
+		current->thread.cet.ibt_bitmap_size = size;
+		bitmap = do_mmap_locked(0, size, PROT_READ | PROT_WRITE,
+					MAP_ANONYMOUS | MAP_PRIVATE,
+					VM_DONTDUMP);
+
+		if (bitmap >= TASK_SIZE) {
+			current->thread.cet.ibt_bitmap_size = 0;
+			return -ENOMEM;
+		}
+
+		current->thread.cet.ibt_bitmap_addr = bitmap;
+	}
+
+	/*
+	 * Lower bits of MSR_IA32_CET_LEG_IW_EN are for IBT
+	 * settings.  Clear lower bits even bitmap is already
+	 * page-aligned.
+	 */
+	bitmap = current->thread.cet.ibt_bitmap_addr;
+	bitmap &= PAGE_MASK;
+
+	/*
+	 * Turn on IBT legacy bitmap.
+	 */
+	rdmsrl(MSR_IA32_U_CET, r);
+	r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
+	wrmsrl(MSR_IA32_U_CET, r);
+	return 0;
+}
-- 
2.17.1

[RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Yu-cheng Yu]

Indirect branch tracking provides an optional legacy code bitmap
that indicates locations of non-IBT compatible code.  When set,
each bit in the bitmap represents a page in the linear address is
legacy code.

We allocate the bitmap only when the application requests it.
Most applications do not need the bitmap.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
index 6adfe795d692..a65d9745af08 100644
--- a/arch/x86/kernel/cet.c
+++ b/arch/x86/kernel/cet.c
@@ -314,3 +314,48 @@ void cet_disable_ibt(void)
 	wrmsrl(MSR_IA32_U_CET, r);
 	current->thread.cet.ibt_enabled = 0;
 }
+
+int cet_setup_ibt_bitmap(void)
+{
+	u64 r;
+	unsigned long bitmap;
+	unsigned long size;
+
+	if (!cpu_feature_enabled(X86_FEATURE_IBT))
+		return -EOPNOTSUPP;
+
+	if (!current->thread.cet.ibt_bitmap_addr) {
+		/*
+		 * Calculate size and put in thread header.
+		 * may_expand_vm() needs this information.
+		 */
+		size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
+		current->thread.cet.ibt_bitmap_size = size;
+		bitmap = do_mmap_locked(0, size, PROT_READ | PROT_WRITE,
+					MAP_ANONYMOUS | MAP_PRIVATE,
+					VM_DONTDUMP);
+
+		if (bitmap >= TASK_SIZE) {
+			current->thread.cet.ibt_bitmap_size = 0;
+			return -ENOMEM;
+		}
+
+		current->thread.cet.ibt_bitmap_addr = bitmap;
+	}
+
+	/*
+	 * Lower bits of MSR_IA32_CET_LEG_IW_EN are for IBT
+	 * settings.  Clear lower bits even bitmap is already
+	 * page-aligned.
+	 */
+	bitmap = current->thread.cet.ibt_bitmap_addr;
+	bitmap &= PAGE_MASK;
+
+	/*
+	 * Turn on IBT legacy bitmap.
+	 */
+	rdmsrl(MSR_IA32_U_CET, r);
+	r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
+	wrmsrl(MSR_IA32_U_CET, r);
+	return 0;
+}
-- 
2.17.1

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Eugene Syromiatnikov]

On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> Indirect branch tracking provides an optional legacy code bitmap
> that indicates locations of non-IBT compatible code.  When set,
> each bit in the bitmap represents a page in the linear address is
> legacy code.
> 
> We allocate the bitmap only when the application requests it.
> Most applications do not need the bitmap.
> 
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
>  arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> index 6adfe795d692..a65d9745af08 100644
> --- a/arch/x86/kernel/cet.c
> +++ b/arch/x86/kernel/cet.c
> @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
>  	wrmsrl(MSR_IA32_U_CET, r);
>  	current->thread.cet.ibt_enabled = 0;
>  }
> +
> +int cet_setup_ibt_bitmap(void)
> +{
> +	u64 r;
> +	unsigned long bitmap;
> +	unsigned long size;
> +
> +	if (!cpu_feature_enabled(X86_FEATURE_IBT))
> +		return -EOPNOTSUPP;
> +
> +	if (!current->thread.cet.ibt_bitmap_addr) {
> +		/*
> +		 * Calculate size and put in thread header.
> +		 * may_expand_vm() needs this information.
> +		 */
> +		size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;

TASK_SIZE_MAX is likely needed here, as an application can easily switch
between long an 32-bit protected mode.  And then the case of a CPU that
doesn't support 5LPT.

> +		current->thread.cet.ibt_bitmap_size = size;
> +		bitmap = do_mmap_locked(0, size, PROT_READ | PROT_WRITE,
> +					MAP_ANONYMOUS | MAP_PRIVATE,
> +					VM_DONTDUMP);
> +
> +		if (bitmap >= TASK_SIZE) {

Shouldn't bitmap be unmapped here?

> +			current->thread.cet.ibt_bitmap_size = 0;
> +			return -ENOMEM;
> +		}
> +
> +		current->thread.cet.ibt_bitmap_addr = bitmap;
> +	}
> +
> +	/*
> +	 * Lower bits of MSR_IA32_CET_LEG_IW_EN are for IBT
> +	 * settings.  Clear lower bits even bitmap is already
> +	 * page-aligned.
> +	 */
> +	bitmap = current->thread.cet.ibt_bitmap_addr;
> +	bitmap &= PAGE_MASK;

In a hypothetical situation of bitmap & PAGE_MASK < bitmap that would lead
to bitmap pointing to unmapped memory. A check that bitmap is sane would
probably be better.

> +
> +	/*
> +	 * Turn on IBT legacy bitmap.
> +	 */
> +	rdmsrl(MSR_IA32_U_CET, r);
> +	r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> +	wrmsrl(MSR_IA32_U_CET, r);
> +	return 0;
> +}
> -- 
> 2.17.1
> 

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Eugene Syromiatnikov]

On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> Indirect branch tracking provides an optional legacy code bitmap
> that indicates locations of non-IBT compatible code.  When set,
> each bit in the bitmap represents a page in the linear address is
> legacy code.
> 
> We allocate the bitmap only when the application requests it.
> Most applications do not need the bitmap.
> 
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
>  arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> index 6adfe795d692..a65d9745af08 100644
> --- a/arch/x86/kernel/cet.c
> +++ b/arch/x86/kernel/cet.c
> @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
>  	wrmsrl(MSR_IA32_U_CET, r);
>  	current->thread.cet.ibt_enabled = 0;
>  }
> +
> +int cet_setup_ibt_bitmap(void)
> +{
> +	u64 r;
> +	unsigned long bitmap;
> +	unsigned long size;
> +
> +	if (!cpu_feature_enabled(X86_FEATURE_IBT))
> +		return -EOPNOTSUPP;
> +
> +	if (!current->thread.cet.ibt_bitmap_addr) {
> +		/*
> +		 * Calculate size and put in thread header.
> +		 * may_expand_vm() needs this information.
> +		 */
> +		size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;

TASK_SIZE_MAX is likely needed here, as an application can easily switch
between long an 32-bit protected mode.  And then the case of a CPU that
doesn't support 5LPT.

> +		current->thread.cet.ibt_bitmap_size = size;
> +		bitmap = do_mmap_locked(0, size, PROT_READ | PROT_WRITE,
> +					MAP_ANONYMOUS | MAP_PRIVATE,
> +					VM_DONTDUMP);
> +
> +		if (bitmap >= TASK_SIZE) {

Shouldn't bitmap be unmapped here?

> +			current->thread.cet.ibt_bitmap_size = 0;
> +			return -ENOMEM;
> +		}
> +
> +		current->thread.cet.ibt_bitmap_addr = bitmap;
> +	}
> +
> +	/*
> +	 * Lower bits of MSR_IA32_CET_LEG_IW_EN are for IBT
> +	 * settings.  Clear lower bits even bitmap is already
> +	 * page-aligned.
> +	 */
> +	bitmap = current->thread.cet.ibt_bitmap_addr;
> +	bitmap &= PAGE_MASK;

In a hypothetical situation of bitmap & PAGE_MASK < bitmap that would lead
to bitmap pointing to unmapped memory. A check that bitmap is sane would
probably be better.

> +
> +	/*
> +	 * Turn on IBT legacy bitmap.
> +	 */
> +	rdmsrl(MSR_IA32_U_CET, r);
> +	r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> +	wrmsrl(MSR_IA32_U_CET, r);
> +	return 0;
> +}
> -- 
> 2.17.1
> 

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Yu-cheng Yu]

On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > Indirect branch tracking provides an optional legacy code bitmap
> > that indicates locations of non-IBT compatible code.  When set,
> > each bit in the bitmap represents a page in the linear address is
> > legacy code.
> > 
> > We allocate the bitmap only when the application requests it.
> > Most applications do not need the bitmap.
> > 
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > ---
> >  arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > index 6adfe795d692..a65d9745af08 100644
> > --- a/arch/x86/kernel/cet.c
> > +++ b/arch/x86/kernel/cet.c
> > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> >  	wrmsrl(MSR_IA32_U_CET, r);
> >  	current->thread.cet.ibt_enabled = 0;
> >  }
> > +
> > +int cet_setup_ibt_bitmap(void)
> > +{
> > +	u64 r;
> > +	unsigned long bitmap;
> > +	unsigned long size;
> > +
> > +	if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > +		return -EOPNOTSUPP;
> > +
> > +	if (!current->thread.cet.ibt_bitmap_addr) {
> > +		/*
> > +		 * Calculate size and put in thread header.
> > +		 * may_expand_vm() needs this information.
> > +		 */
> > +		size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> 
> TASK_SIZE_MAX is likely needed here, as an application can easily switch
> between long an 32-bit protected mode.  And then the case of a CPU that
> doesn't support 5LPT.

If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would have
failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
which is printed from the current code.

Yu-cheng


x64:
TASK_SIZE_MAX	= 0000 7fff ffff f000
TASK_SIZE	= 0000 7fff ffff f000
bitmap size	= 0000 0000 ffff ffff

x32:
TASK_SIZE_MAX	= 0000 7fff ffff f000
TASK_SIZE	= 0000 0000 ffff e000
bitmap size	= 0000 0000 0001 ffff

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Yu-cheng Yu]

On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > Indirect branch tracking provides an optional legacy code bitmap
> > that indicates locations of non-IBT compatible code.  When set,
> > each bit in the bitmap represents a page in the linear address is
> > legacy code.
> > 
> > We allocate the bitmap only when the application requests it.
> > Most applications do not need the bitmap.
> > 
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > ---
> >  arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > index 6adfe795d692..a65d9745af08 100644
> > --- a/arch/x86/kernel/cet.c
> > +++ b/arch/x86/kernel/cet.c
> > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> >  	wrmsrl(MSR_IA32_U_CET, r);
> >  	current->thread.cet.ibt_enabled = 0;
> >  }
> > +
> > +int cet_setup_ibt_bitmap(void)
> > +{
> > +	u64 r;
> > +	unsigned long bitmap;
> > +	unsigned long size;
> > +
> > +	if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > +		return -EOPNOTSUPP;
> > +
> > +	if (!current->thread.cet.ibt_bitmap_addr) {
> > +		/*
> > +		 * Calculate size and put in thread header.
> > +		 * may_expand_vm() needs this information.
> > +		 */
> > +		size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> 
> TASK_SIZE_MAX is likely needed here, as an application can easily switch
> between long an 32-bit protected mode.  And then the case of a CPU that
> doesn't support 5LPT.

If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would have
failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
which is printed from the current code.

Yu-cheng


x64:
TASK_SIZE_MAX	= 0000 7fff ffff f000
TASK_SIZE	= 0000 7fff ffff f000
bitmap size	= 0000 0000 ffff ffff

x32:
TASK_SIZE_MAX	= 0000 7fff ffff f000
TASK_SIZE	= 0000 0000 ffff e000
bitmap size	= 0000 0000 0001 ffff

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Andy Lutomirski]

> On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> 
>> On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
>>> On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
>>> Indirect branch tracking provides an optional legacy code bitmap
>>> that indicates locations of non-IBT compatible code.  When set,
>>> each bit in the bitmap represents a page in the linear address is
>>> legacy code.
>>> 
>>> We allocate the bitmap only when the application requests it.
>>> Most applications do not need the bitmap.
>>> 
>>> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
>>> ---
>>> arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 45 insertions(+)
>>> 
>>> diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
>>> index 6adfe795d692..a65d9745af08 100644
>>> --- a/arch/x86/kernel/cet.c
>>> +++ b/arch/x86/kernel/cet.c
>>> @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
>>>    wrmsrl(MSR_IA32_U_CET, r);
>>>    current->thread.cet.ibt_enabled = 0;
>>> }
>>> +
>>> +int cet_setup_ibt_bitmap(void)
>>> +{
>>> +    u64 r;
>>> +    unsigned long bitmap;
>>> +    unsigned long size;
>>> +
>>> +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
>>> +        return -EOPNOTSUPP;
>>> +
>>> +    if (!current->thread.cet.ibt_bitmap_addr) {
>>> +        /*
>>> +         * Calculate size and put in thread header.
>>> +         * may_expand_vm() needs this information.
>>> +         */
>>> +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
>> 
>> TASK_SIZE_MAX is likely needed here, as an application can easily switch
>> between long an 32-bit protected mode.  And then the case of a CPU that
>> doesn't support 5LPT.
> 
> If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would have
> failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
> which is printed from the current code.
> 
> Yu-cheng
> 
> 
> x64:
> TASK_SIZE_MAX    = 0000 7fff ffff f000
> TASK_SIZE    = 0000 7fff ffff f000
> bitmap size    = 0000 0000 ffff ffff
> 
> x32:
> TASK_SIZE_MAX    = 0000 7fff ffff f000
> TASK_SIZE    = 0000 0000 ffff e000
> bitmap size    = 0000 0000 0001 ffff
> 

I haven’t followed all the details here, but I have a general policy of objecting to any new use of TASK_SIZE. If you really really need to depend on 32-bitness in new code, please figure out what exactly you mean by “32-bit” and use an explicit check.

Some day I would love to delete TASK_SIZE.

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Andy Lutomirski]

> On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> 
>> On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
>>> On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
>>> Indirect branch tracking provides an optional legacy code bitmap
>>> that indicates locations of non-IBT compatible code.  When set,
>>> each bit in the bitmap represents a page in the linear address is
>>> legacy code.
>>> 
>>> We allocate the bitmap only when the application requests it.
>>> Most applications do not need the bitmap.
>>> 
>>> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
>>> ---
>>> arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 45 insertions(+)
>>> 
>>> diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
>>> index 6adfe795d692..a65d9745af08 100644
>>> --- a/arch/x86/kernel/cet.c
>>> +++ b/arch/x86/kernel/cet.c
>>> @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
>>>    wrmsrl(MSR_IA32_U_CET, r);
>>>    current->thread.cet.ibt_enabled = 0;
>>> }
>>> +
>>> +int cet_setup_ibt_bitmap(void)
>>> +{
>>> +    u64 r;
>>> +    unsigned long bitmap;
>>> +    unsigned long size;
>>> +
>>> +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
>>> +        return -EOPNOTSUPP;
>>> +
>>> +    if (!current->thread.cet.ibt_bitmap_addr) {
>>> +        /*
>>> +         * Calculate size and put in thread header.
>>> +         * may_expand_vm() needs this information.
>>> +         */
>>> +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
>> 
>> TASK_SIZE_MAX is likely needed here, as an application can easily switch
>> between long an 32-bit protected mode.  And then the case of a CPU that
>> doesn't support 5LPT.
> 
> If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would have
> failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
> which is printed from the current code.
> 
> Yu-cheng
> 
> 
> x64:
> TASK_SIZE_MAX    = 0000 7fff ffff f000
> TASK_SIZE    = 0000 7fff ffff f000
> bitmap size    = 0000 0000 ffff ffff
> 
> x32:
> TASK_SIZE_MAX    = 0000 7fff ffff f000
> TASK_SIZE    = 0000 0000 ffff e000
> bitmap size    = 0000 0000 0001 ffff
> 

I haven’t followed all the details here, but I have a general policy of objecting to any new use of TASK_SIZE. If you really really need to depend on 32-bitness in new code, please figure out what exactly you mean by “32-bit” and use an explicit check.

Some day I would love to delete TASK_SIZE.

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Yu-cheng Yu]

On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
> > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > 
> > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > > > Indirect branch tracking provides an optional legacy code bitmap
> > > > that indicates locations of non-IBT compatible code.  When set,
> > > > each bit in the bitmap represents a page in the linear address is
> > > > legacy code.
> > > > 
> > > > We allocate the bitmap only when the application requests it.
> > > > Most applications do not need the bitmap.
> > > > 
> > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > ---
> > > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 45 insertions(+)
> > > > 
> > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > > > index 6adfe795d692..a65d9745af08 100644
> > > > --- a/arch/x86/kernel/cet.c
> > > > +++ b/arch/x86/kernel/cet.c
> > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> > > >    wrmsrl(MSR_IA32_U_CET, r);
> > > >    current->thread.cet.ibt_enabled = 0;
> > > > }
> > > > +
> > > > +int cet_setup_ibt_bitmap(void)
> > > > +{
> > > > +    u64 r;
> > > > +    unsigned long bitmap;
> > > > +    unsigned long size;
> > > > +
> > > > +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > > > +        return -EOPNOTSUPP;
> > > > +
> > > > +    if (!current->thread.cet.ibt_bitmap_addr) {
> > > > +        /*
> > > > +         * Calculate size and put in thread header.
> > > > +         * may_expand_vm() needs this information.
> > > > +         */
> > > > +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> > > 
> > > TASK_SIZE_MAX is likely needed here, as an application can easily switch
> > > between long an 32-bit protected mode.  And then the case of a CPU that
> > > doesn't support 5LPT.
> > 
> > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would
> > have
> > failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
> > which is printed from the current code.
> > 
> > Yu-cheng
> > 
> > 
> > x64:
> > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > TASK_SIZE    = 0000 7fff ffff f000
> > bitmap size    = 0000 0000 ffff ffff
> > 
> > x32:
> > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > TASK_SIZE    = 0000 0000 ffff e000
> > bitmap size    = 0000 0000 0001 ffff
> > 
> 
> I haven’t followed all the details here, but I have a general policy of
> objecting to any new use of TASK_SIZE. If you really really need to depend on
> 32-bitness in new code, please figure out what exactly you mean by “32-bit”
> and use an explicit check.

The explicit check would be:

test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX

which is the same as TASK_SIZE.

Or, do we want a new macro?

#define IBT_BITMAP_SIZE (test_thread_flag(TIF_ADDR32) ? \
	(IA32_PAGE_OFFSET / PAGE_SIZE / BITS_PER_BYTE) : \
	(TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE))

Yu-cheng

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Yu-cheng Yu]

On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
> > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > 
> > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > > > Indirect branch tracking provides an optional legacy code bitmap
> > > > that indicates locations of non-IBT compatible code.  When set,
> > > > each bit in the bitmap represents a page in the linear address is
> > > > legacy code.
> > > > 
> > > > We allocate the bitmap only when the application requests it.
> > > > Most applications do not need the bitmap.
> > > > 
> > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > ---
> > > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 45 insertions(+)
> > > > 
> > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > > > index 6adfe795d692..a65d9745af08 100644
> > > > --- a/arch/x86/kernel/cet.c
> > > > +++ b/arch/x86/kernel/cet.c
> > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> > > >    wrmsrl(MSR_IA32_U_CET, r);
> > > >    current->thread.cet.ibt_enabled = 0;
> > > > }
> > > > +
> > > > +int cet_setup_ibt_bitmap(void)
> > > > +{
> > > > +    u64 r;
> > > > +    unsigned long bitmap;
> > > > +    unsigned long size;
> > > > +
> > > > +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > > > +        return -EOPNOTSUPP;
> > > > +
> > > > +    if (!current->thread.cet.ibt_bitmap_addr) {
> > > > +        /*
> > > > +         * Calculate size and put in thread header.
> > > > +         * may_expand_vm() needs this information.
> > > > +         */
> > > > +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> > > 
> > > TASK_SIZE_MAX is likely needed here, as an application can easily switch
> > > between long an 32-bit protected mode.  And then the case of a CPU that
> > > doesn't support 5LPT.
> > 
> > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would
> > have
> > failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
> > which is printed from the current code.
> > 
> > Yu-cheng
> > 
> > 
> > x64:
> > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > TASK_SIZE    = 0000 7fff ffff f000
> > bitmap size    = 0000 0000 ffff ffff
> > 
> > x32:
> > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > TASK_SIZE    = 0000 0000 ffff e000
> > bitmap size    = 0000 0000 0001 ffff
> > 
> 
> I havena??t followed all the details here, but I have a general policy of
> objecting to any new use of TASK_SIZE. If you really really need to depend on
> 32-bitness in new code, please figure out what exactly you mean by a??32-bita??
> and use an explicit check.

The explicit check would be:

test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX

which is the same as TASK_SIZE.

Or, do we want a new macro?

#define IBT_BITMAP_SIZE (test_thread_flag(TIF_ADDR32) ? \
	(IA32_PAGE_OFFSET / PAGE_SIZE / BITS_PER_BYTE) : \
	(TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE))

Yu-cheng

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Yu-cheng Yu]

On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
> > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > 
> > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > > > Indirect branch tracking provides an optional legacy code bitmap
> > > > that indicates locations of non-IBT compatible code.  When set,
> > > > each bit in the bitmap represents a page in the linear address is
> > > > legacy code.
> > > > 
> > > > We allocate the bitmap only when the application requests it.
> > > > Most applications do not need the bitmap.
> > > > 
> > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > ---
> > > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 45 insertions(+)
> > > > 
> > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > > > index 6adfe795d692..a65d9745af08 100644
> > > > --- a/arch/x86/kernel/cet.c
> > > > +++ b/arch/x86/kernel/cet.c
> > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> > > >    wrmsrl(MSR_IA32_U_CET, r);
> > > >    current->thread.cet.ibt_enabled = 0;
> > > > }
> > > > +
> > > > +int cet_setup_ibt_bitmap(void)
> > > > +{
> > > > +    u64 r;
> > > > +    unsigned long bitmap;
> > > > +    unsigned long size;
> > > > +
> > > > +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > > > +        return -EOPNOTSUPP;
> > > > +
> > > > +    if (!current->thread.cet.ibt_bitmap_addr) {
> > > > +        /*
> > > > +         * Calculate size and put in thread header.
> > > > +         * may_expand_vm() needs this information.
> > > > +         */
> > > > +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> > > 
> > > TASK_SIZE_MAX is likely needed here, as an application can easily switch
> > > between long an 32-bit protected mode.  And then the case of a CPU that
> > > doesn't support 5LPT.
> > 
> > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would
> > have
> > failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
> > which is printed from the current code.
> > 
> > Yu-cheng
> > 
> > 
> > x64:
> > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > TASK_SIZE    = 0000 7fff ffff f000
> > bitmap size    = 0000 0000 ffff ffff
> > 
> > x32:
> > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > TASK_SIZE    = 0000 0000 ffff e000
> > bitmap size    = 0000 0000 0001 ffff
> > 
> 
> I haven’t followed all the details here, but I have a general policy of
> objecting to any new use of TASK_SIZE. If you really really need to depend on
> 32-bitness in new code, please figure out what exactly you mean by “32-bit”
> and use an explicit check.

The explicit check would be:

test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX

which is the same as TASK_SIZE.

Or, do we want a new macro?

#define IBT_BITMAP_SIZE (test_thread_flag(TIF_ADDR32) ? \
	(IA32_PAGE_OFFSET / PAGE_SIZE / BITS_PER_BYTE) : \
	(TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE))

Yu-cheng

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Andy Lutomirski]

On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
> > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > >
> > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > > > > Indirect branch tracking provides an optional legacy code bitmap
> > > > > that indicates locations of non-IBT compatible code.  When set,
> > > > > each bit in the bitmap represents a page in the linear address is
> > > > > legacy code.
> > > > >
> > > > > We allocate the bitmap only when the application requests it.
> > > > > Most applications do not need the bitmap.
> > > > >
> > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > > ---
> > > > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> > > > > 1 file changed, 45 insertions(+)
> > > > >
> > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > > > > index 6adfe795d692..a65d9745af08 100644
> > > > > --- a/arch/x86/kernel/cet.c
> > > > > +++ b/arch/x86/kernel/cet.c
> > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> > > > >    wrmsrl(MSR_IA32_U_CET, r);
> > > > >    current->thread.cet.ibt_enabled = 0;
> > > > > }
> > > > > +
> > > > > +int cet_setup_ibt_bitmap(void)
> > > > > +{
> > > > > +    u64 r;
> > > > > +    unsigned long bitmap;
> > > > > +    unsigned long size;
> > > > > +
> > > > > +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > > > > +        return -EOPNOTSUPP;
> > > > > +
> > > > > +    if (!current->thread.cet.ibt_bitmap_addr) {
> > > > > +        /*
> > > > > +         * Calculate size and put in thread header.
> > > > > +         * may_expand_vm() needs this information.
> > > > > +         */
> > > > > +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> > > >
> > > > TASK_SIZE_MAX is likely needed here, as an application can easily switch
> > > > between long an 32-bit protected mode.  And then the case of a CPU that
> > > > doesn't support 5LPT.
> > >
> > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would
> > > have
> > > failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
> > > which is printed from the current code.
> > >
> > > Yu-cheng
> > >
> > >
> > > x64:
> > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > TASK_SIZE    = 0000 7fff ffff f000
> > > bitmap size    = 0000 0000 ffff ffff
> > >
> > > x32:
> > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > TASK_SIZE    = 0000 0000 ffff e000
> > > bitmap size    = 0000 0000 0001 ffff
> > >
> >
> > I haven’t followed all the details here, but I have a general policy of
> > objecting to any new use of TASK_SIZE. If you really really need to depend on
> > 32-bitness in new code, please figure out what exactly you mean by “32-bit”
> > and use an explicit check.
>
> The explicit check would be:
>
> test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX
>
> which is the same as TASK_SIZE.

But this is only ever done in response to a syscall, right?  So
wouldn't in_compat_syscall() be the right check?

Also, this whole thing makes me extremely nervous.  The MSR only
contains the start address, not the size, right?  So what prevents
some goof from causing the CPU to read way past the end of the bitmap
if the bitmap is short because the kernel thought it was supposed to
be 32-bit?

I'm inclined to suggest something awful-ish: always allocate the
bitmap as though it's for a 64-bit process, and just let it be at a
high address.  And add a syscall or arch_prctl() to manipulate it for
the benefit of 32-bit programs that can't address it directly.

>
> Or, do we want a new macro?
>
> #define IBT_BITMAP_SIZE (test_thread_flag(TIF_ADDR32) ? \
>         (IA32_PAGE_OFFSET / PAGE_SIZE / BITS_PER_BYTE) : \
>         (TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE))

No.  I don't like hiding magic like this in a macro that looks like a constant.

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Andy Lutomirski]

On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
> > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > >
> > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > > > > Indirect branch tracking provides an optional legacy code bitmap
> > > > > that indicates locations of non-IBT compatible code.  When set,
> > > > > each bit in the bitmap represents a page in the linear address is
> > > > > legacy code.
> > > > >
> > > > > We allocate the bitmap only when the application requests it.
> > > > > Most applications do not need the bitmap.
> > > > >
> > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > > ---
> > > > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> > > > > 1 file changed, 45 insertions(+)
> > > > >
> > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > > > > index 6adfe795d692..a65d9745af08 100644
> > > > > --- a/arch/x86/kernel/cet.c
> > > > > +++ b/arch/x86/kernel/cet.c
> > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> > > > >    wrmsrl(MSR_IA32_U_CET, r);
> > > > >    current->thread.cet.ibt_enabled = 0;
> > > > > }
> > > > > +
> > > > > +int cet_setup_ibt_bitmap(void)
> > > > > +{
> > > > > +    u64 r;
> > > > > +    unsigned long bitmap;
> > > > > +    unsigned long size;
> > > > > +
> > > > > +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > > > > +        return -EOPNOTSUPP;
> > > > > +
> > > > > +    if (!current->thread.cet.ibt_bitmap_addr) {
> > > > > +        /*
> > > > > +         * Calculate size and put in thread header.
> > > > > +         * may_expand_vm() needs this information.
> > > > > +         */
> > > > > +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> > > >
> > > > TASK_SIZE_MAX is likely needed here, as an application can easily switch
> > > > between long an 32-bit protected mode.  And then the case of a CPU that
> > > > doesn't support 5LPT.
> > >
> > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would
> > > have
> > > failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
> > > which is printed from the current code.
> > >
> > > Yu-cheng
> > >
> > >
> > > x64:
> > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > TASK_SIZE    = 0000 7fff ffff f000
> > > bitmap size    = 0000 0000 ffff ffff
> > >
> > > x32:
> > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > TASK_SIZE    = 0000 0000 ffff e000
> > > bitmap size    = 0000 0000 0001 ffff
> > >
> >
> > I haven’t followed all the details here, but I have a general policy of
> > objecting to any new use of TASK_SIZE. If you really really need to depend on
> > 32-bitness in new code, please figure out what exactly you mean by “32-bit”
> > and use an explicit check.
>
> The explicit check would be:
>
> test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX
>
> which is the same as TASK_SIZE.

But this is only ever done in response to a syscall, right?  So
wouldn't in_compat_syscall() be the right check?

Also, this whole thing makes me extremely nervous.  The MSR only
contains the start address, not the size, right?  So what prevents
some goof from causing the CPU to read way past the end of the bitmap
if the bitmap is short because the kernel thought it was supposed to
be 32-bit?

I'm inclined to suggest something awful-ish: always allocate the
bitmap as though it's for a 64-bit process, and just let it be at a
high address.  And add a syscall or arch_prctl() to manipulate it for
the benefit of 32-bit programs that can't address it directly.

>
> Or, do we want a new macro?
>
> #define IBT_BITMAP_SIZE (test_thread_flag(TIF_ADDR32) ? \
>         (IA32_PAGE_OFFSET / PAGE_SIZE / BITS_PER_BYTE) : \
>         (TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE))

No.  I don't like hiding magic like this in a macro that looks like a constant.

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Eugene Syromiatnikov]

On Fri, Oct 05, 2018 at 10:07:46AM -0700, Andy Lutomirski wrote:
> On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> >
> > On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
> > > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > >
> > > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> > > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > > > > > Indirect branch tracking provides an optional legacy code bitmap
> > > > > > that indicates locations of non-IBT compatible code.  When set,
> > > > > > each bit in the bitmap represents a page in the linear address is
> > > > > > legacy code.
> > > > > >
> > > > > > We allocate the bitmap only when the application requests it.
> > > > > > Most applications do not need the bitmap.
> > > > > >
> > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > > > ---
> > > > > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> > > > > > 1 file changed, 45 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > > > > > index 6adfe795d692..a65d9745af08 100644
> > > > > > --- a/arch/x86/kernel/cet.c
> > > > > > +++ b/arch/x86/kernel/cet.c
> > > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> > > > > >    wrmsrl(MSR_IA32_U_CET, r);
> > > > > >    current->thread.cet.ibt_enabled = 0;
> > > > > > }
> > > > > > +
> > > > > > +int cet_setup_ibt_bitmap(void)
> > > > > > +{
> > > > > > +    u64 r;
> > > > > > +    unsigned long bitmap;
> > > > > > +    unsigned long size;
> > > > > > +
> > > > > > +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > > > > > +        return -EOPNOTSUPP;
> > > > > > +
> > > > > > +    if (!current->thread.cet.ibt_bitmap_addr) {
> > > > > > +        /*
> > > > > > +         * Calculate size and put in thread header.
> > > > > > +         * may_expand_vm() needs this information.
> > > > > > +         */
> > > > > > +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> > > > >
> > > > > TASK_SIZE_MAX is likely needed here, as an application can easily switch
> > > > > between long an 32-bit protected mode.  And then the case of a CPU that
> > > > > doesn't support 5LPT.
> > > >
> > > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would
> > > > have
> > > > failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
> > > > which is printed from the current code.
> > > >
> > > > Yu-cheng
> > > >
> > > >
> > > > x64:
> > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > TASK_SIZE    = 0000 7fff ffff f000
> > > > bitmap size    = 0000 0000 ffff ffff
> > > >
> > > > x32:
> > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > TASK_SIZE    = 0000 0000 ffff e000
> > > > bitmap size    = 0000 0000 0001 ffff
> > > >
> > >
> > > I haven’t followed all the details here, but I have a general policy of
> > > objecting to any new use of TASK_SIZE. If you really really need to depend on
> > > 32-bitness in new code, please figure out what exactly you mean by “32-bit”
> > > and use an explicit check.
> >
> > The explicit check would be:
> >
> > test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX
> >
> > which is the same as TASK_SIZE.
> 
> But this is only ever done in response to a syscall, right?  So
> wouldn't in_compat_syscall() be the right check?
> 
> Also, this whole thing makes me extremely nervous.  The MSR only
> contains the start address, not the size, right?  So what prevents
> some goof from causing the CPU to read way past the end of the bitmap
> if the bitmap is short because the kernel thought it was supposed to
> be 32-bit?

That's what I've mentioned initially: every syscall made with int 0x80
is interpreted as compat, even if it was made from long mode.

> I'm inclined to suggest something awful-ish: always allocate the
> bitmap as though it's for a 64-bit process, and just let it be at a
> high address.  And add a syscall or arch_prctl() to manipulate it for
> the benefit of 32-bit programs that can't address it directly.

That's likely the only way to go.

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Eugene Syromiatnikov]

On Fri, Oct 05, 2018 at 10:07:46AM -0700, Andy Lutomirski wrote:
> On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> >
> > On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
> > > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > >
> > > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> > > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > > > > > Indirect branch tracking provides an optional legacy code bitmap
> > > > > > that indicates locations of non-IBT compatible code.  When set,
> > > > > > each bit in the bitmap represents a page in the linear address is
> > > > > > legacy code.
> > > > > >
> > > > > > We allocate the bitmap only when the application requests it.
> > > > > > Most applications do not need the bitmap.
> > > > > >
> > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > > > ---
> > > > > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> > > > > > 1 file changed, 45 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > > > > > index 6adfe795d692..a65d9745af08 100644
> > > > > > --- a/arch/x86/kernel/cet.c
> > > > > > +++ b/arch/x86/kernel/cet.c
> > > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> > > > > >    wrmsrl(MSR_IA32_U_CET, r);
> > > > > >    current->thread.cet.ibt_enabled = 0;
> > > > > > }
> > > > > > +
> > > > > > +int cet_setup_ibt_bitmap(void)
> > > > > > +{
> > > > > > +    u64 r;
> > > > > > +    unsigned long bitmap;
> > > > > > +    unsigned long size;
> > > > > > +
> > > > > > +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > > > > > +        return -EOPNOTSUPP;
> > > > > > +
> > > > > > +    if (!current->thread.cet.ibt_bitmap_addr) {
> > > > > > +        /*
> > > > > > +         * Calculate size and put in thread header.
> > > > > > +         * may_expand_vm() needs this information.
> > > > > > +         */
> > > > > > +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> > > > >
> > > > > TASK_SIZE_MAX is likely needed here, as an application can easily switch
> > > > > between long an 32-bit protected mode.  And then the case of a CPU that
> > > > > doesn't support 5LPT.
> > > >
> > > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would
> > > > have
> > > > failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
> > > > which is printed from the current code.
> > > >
> > > > Yu-cheng
> > > >
> > > >
> > > > x64:
> > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > TASK_SIZE    = 0000 7fff ffff f000
> > > > bitmap size    = 0000 0000 ffff ffff
> > > >
> > > > x32:
> > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > TASK_SIZE    = 0000 0000 ffff e000
> > > > bitmap size    = 0000 0000 0001 ffff
> > > >
> > >
> > > I havena??t followed all the details here, but I have a general policy of
> > > objecting to any new use of TASK_SIZE. If you really really need to depend on
> > > 32-bitness in new code, please figure out what exactly you mean by a??32-bita??
> > > and use an explicit check.
> >
> > The explicit check would be:
> >
> > test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX
> >
> > which is the same as TASK_SIZE.
> 
> But this is only ever done in response to a syscall, right?  So
> wouldn't in_compat_syscall() be the right check?
> 
> Also, this whole thing makes me extremely nervous.  The MSR only
> contains the start address, not the size, right?  So what prevents
> some goof from causing the CPU to read way past the end of the bitmap
> if the bitmap is short because the kernel thought it was supposed to
> be 32-bit?

That's what I've mentioned initially: every syscall made with int 0x80
is interpreted as compat, even if it was made from long mode.

> I'm inclined to suggest something awful-ish: always allocate the
> bitmap as though it's for a 64-bit process, and just let it be at a
> high address.  And add a syscall or arch_prctl() to manipulate it for
> the benefit of 32-bit programs that can't address it directly.

That's likely the only way to go.

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Eugene Syromiatnikov]

On Fri, Oct 05, 2018 at 10:07:46AM -0700, Andy Lutomirski wrote:
> On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> >
> > On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
> > > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > >
> > > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> > > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > > > > > Indirect branch tracking provides an optional legacy code bitmap
> > > > > > that indicates locations of non-IBT compatible code.  When set,
> > > > > > each bit in the bitmap represents a page in the linear address is
> > > > > > legacy code.
> > > > > >
> > > > > > We allocate the bitmap only when the application requests it.
> > > > > > Most applications do not need the bitmap.
> > > > > >
> > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > > > ---
> > > > > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> > > > > > 1 file changed, 45 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > > > > > index 6adfe795d692..a65d9745af08 100644
> > > > > > --- a/arch/x86/kernel/cet.c
> > > > > > +++ b/arch/x86/kernel/cet.c
> > > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> > > > > >    wrmsrl(MSR_IA32_U_CET, r);
> > > > > >    current->thread.cet.ibt_enabled = 0;
> > > > > > }
> > > > > > +
> > > > > > +int cet_setup_ibt_bitmap(void)
> > > > > > +{
> > > > > > +    u64 r;
> > > > > > +    unsigned long bitmap;
> > > > > > +    unsigned long size;
> > > > > > +
> > > > > > +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > > > > > +        return -EOPNOTSUPP;
> > > > > > +
> > > > > > +    if (!current->thread.cet.ibt_bitmap_addr) {
> > > > > > +        /*
> > > > > > +         * Calculate size and put in thread header.
> > > > > > +         * may_expand_vm() needs this information.
> > > > > > +         */
> > > > > > +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> > > > >
> > > > > TASK_SIZE_MAX is likely needed here, as an application can easily switch
> > > > > between long an 32-bit protected mode.  And then the case of a CPU that
> > > > > doesn't support 5LPT.
> > > >
> > > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would
> > > > have
> > > > failed the allocation for bitmap size > TASK_SIZE.  Please see values below,
> > > > which is printed from the current code.
> > > >
> > > > Yu-cheng
> > > >
> > > >
> > > > x64:
> > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > TASK_SIZE    = 0000 7fff ffff f000
> > > > bitmap size    = 0000 0000 ffff ffff
> > > >
> > > > x32:
> > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > TASK_SIZE    = 0000 0000 ffff e000
> > > > bitmap size    = 0000 0000 0001 ffff
> > > >
> > >
> > > I haven’t followed all the details here, but I have a general policy of
> > > objecting to any new use of TASK_SIZE. If you really really need to depend on
> > > 32-bitness in new code, please figure out what exactly you mean by “32-bit”
> > > and use an explicit check.
> >
> > The explicit check would be:
> >
> > test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX
> >
> > which is the same as TASK_SIZE.
> 
> But this is only ever done in response to a syscall, right?  So
> wouldn't in_compat_syscall() be the right check?
> 
> Also, this whole thing makes me extremely nervous.  The MSR only
> contains the start address, not the size, right?  So what prevents
> some goof from causing the CPU to read way past the end of the bitmap
> if the bitmap is short because the kernel thought it was supposed to
> be 32-bit?

That's what I've mentioned initially: every syscall made with int 0x80
is interpreted as compat, even if it was made from long mode.

> I'm inclined to suggest something awful-ish: always allocate the
> bitmap as though it's for a 64-bit process, and just let it be at a
> high address.  And add a syscall or arch_prctl() to manipulate it for
> the benefit of 32-bit programs that can't address it directly.

That's likely the only way to go.

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Yu-cheng Yu]

On Fri, 2018-10-05 at 10:26 -0700, Eugene Syromiatnikov wrote:
> On Fri, Oct 05, 2018 at 10:07:46AM -0700, Andy Lutomirski wrote:
> > On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > 
> > > On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
> > > > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > > > 
> > > > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> > > > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > > > > > > Indirect branch tracking provides an optional legacy code bitmap
> > > > > > > that indicates locations of non-IBT compatible code.  When set,
> > > > > > > each bit in the bitmap represents a page in the linear address is
> > > > > > > legacy code.
> > > > > > > 
> > > > > > > We allocate the bitmap only when the application requests it.
> > > > > > > Most applications do not need the bitmap.
> > > > > > > 
> > > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > > > > ---
> > > > > > > arch/x86/kernel/cet.c | 45
> > > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > > > 1 file changed, 45 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > > > > > > index 6adfe795d692..a65d9745af08 100644
> > > > > > > --- a/arch/x86/kernel/cet.c
> > > > > > > +++ b/arch/x86/kernel/cet.c
> > > > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> > > > > > >    wrmsrl(MSR_IA32_U_CET, r);
> > > > > > >    current->thread.cet.ibt_enabled = 0;
> > > > > > > }
> > > > > > > +
> > > > > > > +int cet_setup_ibt_bitmap(void)
> > > > > > > +{
> > > > > > > +    u64 r;
> > > > > > > +    unsigned long bitmap;
> > > > > > > +    unsigned long size;
> > > > > > > +
> > > > > > > +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > > > > > > +        return -EOPNOTSUPP;
> > > > > > > +
> > > > > > > +    if (!current->thread.cet.ibt_bitmap_addr) {
> > > > > > > +        /*
> > > > > > > +         * Calculate size and put in thread header.
> > > > > > > +         * may_expand_vm() needs this information.
> > > > > > > +         */
> > > > > > > +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> > > > > > 
> > > > > > TASK_SIZE_MAX is likely needed here, as an application can easily
> > > > > > switch
> > > > > > between long an 32-bit protected mode.  And then the case of a CPU
> > > > > > that
> > > > > > doesn't support 5LPT.
> > > > > 
> > > > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps
> > > > > would
> > > > > have
> > > > > failed the allocation for bitmap size > TASK_SIZE.  Please see values
> > > > > below,
> > > > > which is printed from the current code.
> > > > > 
> > > > > Yu-cheng
> > > > > 
> > > > > 
> > > > > x64:
> > > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > > TASK_SIZE    = 0000 7fff ffff f000
> > > > > bitmap size    = 0000 0000 ffff ffff
> > > > > 
> > > > > x32:
> > > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > > TASK_SIZE    = 0000 0000 ffff e000
> > > > > bitmap size    = 0000 0000 0001 ffff
> > > > > 
> > > > 
> > > > I haven’t followed all the details here, but I have a general policy of
> > > > objecting to any new use of TASK_SIZE. If you really really need to
> > > > depend on
> > > > 32-bitness in new code, please figure out what exactly you mean by “32-
> > > > bit”
> > > > and use an explicit check.
> > > 
> > > The explicit check would be:
> > > 
> > > test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX
> > > 
> > > which is the same as TASK_SIZE.
> > 
> > But this is only ever done in response to a syscall, right?  So
> > wouldn't in_compat_syscall() be the right check?
> > 
> > Also, this whole thing makes me extremely nervous.  The MSR only
> > contains the start address, not the size, right?  So what prevents
> > some goof from causing the CPU to read way past the end of the bitmap
> > if the bitmap is short because the kernel thought it was supposed to
> > be 32-bit?
> 
> That's what I've mentioned initially: every syscall made with int 0x80
> is interpreted as compat, even if it was made from long mode.
> 
> > I'm inclined to suggest something awful-ish: always allocate the
> > bitmap as though it's for a 64-bit process, and just let it be at a
> > high address.  And add a syscall or arch_prctl() to manipulate it for
> > the benefit of 32-bit programs that can't address it directly.
> 
> That's likely the only way to go.

This bitmap is needed only when the app does dlopen() a non-IBT .so file.  Most
applications do not need it.  Can't we let dlopen mmap() the bitmap when needed
and pass it to the kernel?

Yu-cheng

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Yu-cheng Yu]

On Fri, 2018-10-05 at 10:26 -0700, Eugene Syromiatnikov wrote:
> On Fri, Oct 05, 2018 at 10:07:46AM -0700, Andy Lutomirski wrote:
> > On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > 
> > > On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
> > > > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > > > 
> > > > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> > > > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > > > > > > Indirect branch tracking provides an optional legacy code bitmap
> > > > > > > that indicates locations of non-IBT compatible code.  When set,
> > > > > > > each bit in the bitmap represents a page in the linear address is
> > > > > > > legacy code.
> > > > > > > 
> > > > > > > We allocate the bitmap only when the application requests it.
> > > > > > > Most applications do not need the bitmap.
> > > > > > > 
> > > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > > > > ---
> > > > > > > arch/x86/kernel/cet.c | 45
> > > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > > > 1 file changed, 45 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > > > > > > index 6adfe795d692..a65d9745af08 100644
> > > > > > > --- a/arch/x86/kernel/cet.c
> > > > > > > +++ b/arch/x86/kernel/cet.c
> > > > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> > > > > > >    wrmsrl(MSR_IA32_U_CET, r);
> > > > > > >    current->thread.cet.ibt_enabled = 0;
> > > > > > > }
> > > > > > > +
> > > > > > > +int cet_setup_ibt_bitmap(void)
> > > > > > > +{
> > > > > > > +    u64 r;
> > > > > > > +    unsigned long bitmap;
> > > > > > > +    unsigned long size;
> > > > > > > +
> > > > > > > +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > > > > > > +        return -EOPNOTSUPP;
> > > > > > > +
> > > > > > > +    if (!current->thread.cet.ibt_bitmap_addr) {
> > > > > > > +        /*
> > > > > > > +         * Calculate size and put in thread header.
> > > > > > > +         * may_expand_vm() needs this information.
> > > > > > > +         */
> > > > > > > +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> > > > > > 
> > > > > > TASK_SIZE_MAX is likely needed here, as an application can easily
> > > > > > switch
> > > > > > between long an 32-bit protected mode.  And then the case of a CPU
> > > > > > that
> > > > > > doesn't support 5LPT.
> > > > > 
> > > > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps
> > > > > would
> > > > > have
> > > > > failed the allocation for bitmap size > TASK_SIZE.  Please see values
> > > > > below,
> > > > > which is printed from the current code.
> > > > > 
> > > > > Yu-cheng
> > > > > 
> > > > > 
> > > > > x64:
> > > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > > TASK_SIZE    = 0000 7fff ffff f000
> > > > > bitmap size    = 0000 0000 ffff ffff
> > > > > 
> > > > > x32:
> > > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > > TASK_SIZE    = 0000 0000 ffff e000
> > > > > bitmap size    = 0000 0000 0001 ffff
> > > > > 
> > > > 
> > > > I havena??t followed all the details here, but I have a general policy of
> > > > objecting to any new use of TASK_SIZE. If you really really need to
> > > > depend on
> > > > 32-bitness in new code, please figure out what exactly you mean by a??32-
> > > > bita??
> > > > and use an explicit check.
> > > 
> > > The explicit check would be:
> > > 
> > > test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX
> > > 
> > > which is the same as TASK_SIZE.
> > 
> > But this is only ever done in response to a syscall, right?  So
> > wouldn't in_compat_syscall() be the right check?
> > 
> > Also, this whole thing makes me extremely nervous.  The MSR only
> > contains the start address, not the size, right?  So what prevents
> > some goof from causing the CPU to read way past the end of the bitmap
> > if the bitmap is short because the kernel thought it was supposed to
> > be 32-bit?
> 
> That's what I've mentioned initially: every syscall made with int 0x80
> is interpreted as compat, even if it was made from long mode.
> 
> > I'm inclined to suggest something awful-ish: always allocate the
> > bitmap as though it's for a 64-bit process, and just let it be at a
> > high address.  And add a syscall or arch_prctl() to manipulate it for
> > the benefit of 32-bit programs that can't address it directly.
> 
> That's likely the only way to go.

This bitmap is needed only when the app does dlopen() a non-IBT .so file.  Most
applications do not need it.  Can't we let dlopen mmap() the bitmap when needed
and pass it to the kernel?

Yu-cheng

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Yu-cheng Yu]

On Fri, 2018-10-05 at 10:26 -0700, Eugene Syromiatnikov wrote:
> On Fri, Oct 05, 2018 at 10:07:46AM -0700, Andy Lutomirski wrote:
> > On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > 
> > > On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
> > > > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > > > 
> > > > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
> > > > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
> > > > > > > Indirect branch tracking provides an optional legacy code bitmap
> > > > > > > that indicates locations of non-IBT compatible code.  When set,
> > > > > > > each bit in the bitmap represents a page in the linear address is
> > > > > > > legacy code.
> > > > > > > 
> > > > > > > We allocate the bitmap only when the application requests it.
> > > > > > > Most applications do not need the bitmap.
> > > > > > > 
> > > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > > > > > ---
> > > > > > > arch/x86/kernel/cet.c | 45
> > > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > > > 1 file changed, 45 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> > > > > > > index 6adfe795d692..a65d9745af08 100644
> > > > > > > --- a/arch/x86/kernel/cet.c
> > > > > > > +++ b/arch/x86/kernel/cet.c
> > > > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
> > > > > > >    wrmsrl(MSR_IA32_U_CET, r);
> > > > > > >    current->thread.cet.ibt_enabled = 0;
> > > > > > > }
> > > > > > > +
> > > > > > > +int cet_setup_ibt_bitmap(void)
> > > > > > > +{
> > > > > > > +    u64 r;
> > > > > > > +    unsigned long bitmap;
> > > > > > > +    unsigned long size;
> > > > > > > +
> > > > > > > +    if (!cpu_feature_enabled(X86_FEATURE_IBT))
> > > > > > > +        return -EOPNOTSUPP;
> > > > > > > +
> > > > > > > +    if (!current->thread.cet.ibt_bitmap_addr) {
> > > > > > > +        /*
> > > > > > > +         * Calculate size and put in thread header.
> > > > > > > +         * may_expand_vm() needs this information.
> > > > > > > +         */
> > > > > > > +        size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> > > > > > 
> > > > > > TASK_SIZE_MAX is likely needed here, as an application can easily
> > > > > > switch
> > > > > > between long an 32-bit protected mode.  And then the case of a CPU
> > > > > > that
> > > > > > doesn't support 5LPT.
> > > > > 
> > > > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps
> > > > > would
> > > > > have
> > > > > failed the allocation for bitmap size > TASK_SIZE.  Please see values
> > > > > below,
> > > > > which is printed from the current code.
> > > > > 
> > > > > Yu-cheng
> > > > > 
> > > > > 
> > > > > x64:
> > > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > > TASK_SIZE    = 0000 7fff ffff f000
> > > > > bitmap size    = 0000 0000 ffff ffff
> > > > > 
> > > > > x32:
> > > > > TASK_SIZE_MAX    = 0000 7fff ffff f000
> > > > > TASK_SIZE    = 0000 0000 ffff e000
> > > > > bitmap size    = 0000 0000 0001 ffff
> > > > > 
> > > > 
> > > > I haven’t followed all the details here, but I have a general policy of
> > > > objecting to any new use of TASK_SIZE. If you really really need to
> > > > depend on
> > > > 32-bitness in new code, please figure out what exactly you mean by “32-
> > > > bit”
> > > > and use an explicit check.
> > > 
> > > The explicit check would be:
> > > 
> > > test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX
> > > 
> > > which is the same as TASK_SIZE.
> > 
> > But this is only ever done in response to a syscall, right?  So
> > wouldn't in_compat_syscall() be the right check?
> > 
> > Also, this whole thing makes me extremely nervous.  The MSR only
> > contains the start address, not the size, right?  So what prevents
> > some goof from causing the CPU to read way past the end of the bitmap
> > if the bitmap is short because the kernel thought it was supposed to
> > be 32-bit?
> 
> That's what I've mentioned initially: every syscall made with int 0x80
> is interpreted as compat, even if it was made from long mode.
> 
> > I'm inclined to suggest something awful-ish: always allocate the
> > bitmap as though it's for a 64-bit process, and just let it be at a
> > high address.  And add a syscall or arch_prctl() to manipulate it for
> > the benefit of 32-bit programs that can't address it directly.
> 
> That's likely the only way to go.

This bitmap is needed only when the app does dlopen() a non-IBT .so file.  Most
applications do not need it.  Can't we let dlopen mmap() the bitmap when needed
and pass it to the kernel?

Yu-cheng

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Andy Lutomirski]

On Fri, Sep 21, 2018 at 8:10 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> Indirect branch tracking provides an optional legacy code bitmap
> that indicates locations of non-IBT compatible code.  When set,
> each bit in the bitmap represents a page in the linear address is
> legacy code.
>
> We allocate the bitmap only when the application requests it.
> Most applications do not need the bitmap.
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
>  arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> index 6adfe795d692..a65d9745af08 100644
> --- a/arch/x86/kernel/cet.c
> +++ b/arch/x86/kernel/cet.c
> @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
>         wrmsrl(MSR_IA32_U_CET, r);
>         current->thread.cet.ibt_enabled = 0;
>  }
> +
> +int cet_setup_ibt_bitmap(void)
> +{
> +       u64 r;
> +       unsigned long bitmap;
> +       unsigned long size;
> +
> +       if (!cpu_feature_enabled(X86_FEATURE_IBT))
> +               return -EOPNOTSUPP;
> +
> +       if (!current->thread.cet.ibt_bitmap_addr) {
> +               /*
> +                * Calculate size and put in thread header.
> +                * may_expand_vm() needs this information.
> +                */
> +               size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> +               current->thread.cet.ibt_bitmap_size = size;
> +               bitmap = do_mmap_locked(0, size, PROT_READ | PROT_WRITE,
> +                                       MAP_ANONYMOUS | MAP_PRIVATE,
> +                                       VM_DONTDUMP);
> +
> +               if (bitmap >= TASK_SIZE) {
> +                       current->thread.cet.ibt_bitmap_size = 0;
> +                       return -ENOMEM;
> +               }
> +
> +               current->thread.cet.ibt_bitmap_addr = bitmap;
> +       }
> +
> +       /*
> +        * Lower bits of MSR_IA32_CET_LEG_IW_EN are for IBT
> +        * settings.  Clear lower bits even bitmap is already
> +        * page-aligned.
> +        */
> +       bitmap = current->thread.cet.ibt_bitmap_addr;
> +       bitmap &= PAGE_MASK;
> +
> +       /*
> +        * Turn on IBT legacy bitmap.
> +        */
> +       rdmsrl(MSR_IA32_U_CET, r);
> +       r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> +       wrmsrl(MSR_IA32_U_CET, r);
> +       return 0;

Why are you writing the MSRs in the case where the bitmap was already allocated?

Re: [RFC PATCH v4 3/9] x86/cet/ibt: Add IBT legacy code bitmap allocation function

[Andy Lutomirski]

On Fri, Sep 21, 2018 at 8:10 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> Indirect branch tracking provides an optional legacy code bitmap
> that indicates locations of non-IBT compatible code.  When set,
> each bit in the bitmap represents a page in the linear address is
> legacy code.
>
> We allocate the bitmap only when the application requests it.
> Most applications do not need the bitmap.
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
>  arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
> index 6adfe795d692..a65d9745af08 100644
> --- a/arch/x86/kernel/cet.c
> +++ b/arch/x86/kernel/cet.c
> @@ -314,3 +314,48 @@ void cet_disable_ibt(void)
>         wrmsrl(MSR_IA32_U_CET, r);
>         current->thread.cet.ibt_enabled = 0;
>  }
> +
> +int cet_setup_ibt_bitmap(void)
> +{
> +       u64 r;
> +       unsigned long bitmap;
> +       unsigned long size;
> +
> +       if (!cpu_feature_enabled(X86_FEATURE_IBT))
> +               return -EOPNOTSUPP;
> +
> +       if (!current->thread.cet.ibt_bitmap_addr) {
> +               /*
> +                * Calculate size and put in thread header.
> +                * may_expand_vm() needs this information.
> +                */
> +               size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE;
> +               current->thread.cet.ibt_bitmap_size = size;
> +               bitmap = do_mmap_locked(0, size, PROT_READ | PROT_WRITE,
> +                                       MAP_ANONYMOUS | MAP_PRIVATE,
> +                                       VM_DONTDUMP);
> +
> +               if (bitmap >= TASK_SIZE) {
> +                       current->thread.cet.ibt_bitmap_size = 0;
> +                       return -ENOMEM;
> +               }
> +
> +               current->thread.cet.ibt_bitmap_addr = bitmap;
> +       }
> +
> +       /*
> +        * Lower bits of MSR_IA32_CET_LEG_IW_EN are for IBT
> +        * settings.  Clear lower bits even bitmap is already
> +        * page-aligned.
> +        */
> +       bitmap = current->thread.cet.ibt_bitmap_addr;
> +       bitmap &= PAGE_MASK;
> +
> +       /*
> +        * Turn on IBT legacy bitmap.
> +        */
> +       rdmsrl(MSR_IA32_U_CET, r);
> +       r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> +       wrmsrl(MSR_IA32_U_CET, r);
> +       return 0;

Why are you writing the MSRs in the case where the bitmap was already allocated?

[RFC PATCH v4 4/9] mm/mmap: Add IBT bitmap size to address space limit check

[Yu-cheng Yu]

The indirect branch tracking legacy bitmap takes a large address
space.  This causes may_expand_vm() failure on the address limit
check.  For a IBT-enabled task, add the bitmap size to the
address limit.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/include/uapi/asm/resource.h |  5 +++++
 include/uapi/asm-generic/resource.h  |  3 +++
 mm/mmap.c                            | 12 +++++++++++-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/resource.h b/arch/x86/include/uapi/asm/resource.h
index 04bc4db8921b..0741b2a6101a 100644
--- a/arch/x86/include/uapi/asm/resource.h
+++ b/arch/x86/include/uapi/asm/resource.h
@@ -1 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+#ifdef CONFIG_X86_INTEL_CET
+#define rlimit_as_extra() current->thread.cet.ibt_bitmap_size
+#endif
+
 #include <asm-generic/resource.h>
diff --git a/include/uapi/asm-generic/resource.h b/include/uapi/asm-generic/resource.h
index f12db7a0da64..8a7608a09700 100644
--- a/include/uapi/asm-generic/resource.h
+++ b/include/uapi/asm-generic/resource.h
@@ -58,5 +58,8 @@
 # define RLIM_INFINITY		(~0UL)
 #endif
 
+#ifndef rlimit_as_extra
+#define rlimit_as_extra() 0
+#endif
 
 #endif /* _UAPI_ASM_GENERIC_RESOURCE_H */
diff --git a/mm/mmap.c b/mm/mmap.c
index fa581ced3f56..397b8cb0b0af 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3237,7 +3237,17 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
  */
 bool may_expand_vm(struct mm_struct *mm, vm_flags_t flags, unsigned long npages)
 {
-	if (mm->total_vm + npages > rlimit(RLIMIT_AS) >> PAGE_SHIFT)
+	unsigned long as_limit = rlimit(RLIMIT_AS);
+	unsigned long as_limit_plus = as_limit + rlimit_as_extra();
+
+	/* as_limit_plus overflowed */
+	if (as_limit_plus < as_limit)
+		as_limit_plus = RLIM_INFINITY;
+
+	if (as_limit_plus > as_limit)
+		as_limit = as_limit_plus;
+
+	if (mm->total_vm + npages > as_limit >> PAGE_SHIFT)
 		return false;
 
 	if (is_data_mapping(flags) &&
-- 
2.17.1

[RFC PATCH v4 4/9] mm/mmap: Add IBT bitmap size to address space limit check

[Yu-cheng Yu]

The indirect branch tracking legacy bitmap takes a large address
space.  This causes may_expand_vm() failure on the address limit
check.  For a IBT-enabled task, add the bitmap size to the
address limit.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/include/uapi/asm/resource.h |  5 +++++
 include/uapi/asm-generic/resource.h  |  3 +++
 mm/mmap.c                            | 12 +++++++++++-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/resource.h b/arch/x86/include/uapi/asm/resource.h
index 04bc4db8921b..0741b2a6101a 100644
--- a/arch/x86/include/uapi/asm/resource.h
+++ b/arch/x86/include/uapi/asm/resource.h
@@ -1 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+#ifdef CONFIG_X86_INTEL_CET
+#define rlimit_as_extra() current->thread.cet.ibt_bitmap_size
+#endif
+
 #include <asm-generic/resource.h>
diff --git a/include/uapi/asm-generic/resource.h b/include/uapi/asm-generic/resource.h
index f12db7a0da64..8a7608a09700 100644
--- a/include/uapi/asm-generic/resource.h
+++ b/include/uapi/asm-generic/resource.h
@@ -58,5 +58,8 @@
 # define RLIM_INFINITY		(~0UL)
 #endif
 
+#ifndef rlimit_as_extra
+#define rlimit_as_extra() 0
+#endif
 
 #endif /* _UAPI_ASM_GENERIC_RESOURCE_H */
diff --git a/mm/mmap.c b/mm/mmap.c
index fa581ced3f56..397b8cb0b0af 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3237,7 +3237,17 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
  */
 bool may_expand_vm(struct mm_struct *mm, vm_flags_t flags, unsigned long npages)
 {
-	if (mm->total_vm + npages > rlimit(RLIMIT_AS) >> PAGE_SHIFT)
+	unsigned long as_limit = rlimit(RLIMIT_AS);
+	unsigned long as_limit_plus = as_limit + rlimit_as_extra();
+
+	/* as_limit_plus overflowed */
+	if (as_limit_plus < as_limit)
+		as_limit_plus = RLIM_INFINITY;
+
+	if (as_limit_plus > as_limit)
+		as_limit = as_limit_plus;
+
+	if (mm->total_vm + npages > as_limit >> PAGE_SHIFT)
 		return false;
 
 	if (is_data_mapping(flags) &&
-- 
2.17.1

Re: [RFC PATCH v4 4/9] mm/mmap: Add IBT bitmap size to address space limit check

[Eugene Syromiatnikov]

On Fri, Sep 21, 2018 at 08:05:48AM -0700, Yu-cheng Yu wrote:
> The indirect branch tracking legacy bitmap takes a large address
> space.  This causes may_expand_vm() failure on the address limit
> check.  For a IBT-enabled task, add the bitmap size to the
> address limit.
> 
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
>  arch/x86/include/uapi/asm/resource.h |  5 +++++
>  include/uapi/asm-generic/resource.h  |  3 +++
>  mm/mmap.c                            | 12 +++++++++++-
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/uapi/asm/resource.h b/arch/x86/include/uapi/asm/resource.h
> index 04bc4db8921b..0741b2a6101a 100644
> --- a/arch/x86/include/uapi/asm/resource.h
> +++ b/arch/x86/include/uapi/asm/resource.h
> @@ -1 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> +#ifdef CONFIG_X86_INTEL_CET
> +#define rlimit_as_extra() current->thread.cet.ibt_bitmap_size
> +#endif

Does this really belong to UAPI?

> +
>  #include <asm-generic/resource.h>
> diff --git a/include/uapi/asm-generic/resource.h b/include/uapi/asm-generic/resource.h
> index f12db7a0da64..8a7608a09700 100644
> --- a/include/uapi/asm-generic/resource.h
> +++ b/include/uapi/asm-generic/resource.h
> @@ -58,5 +58,8 @@
>  # define RLIM_INFINITY		(~0UL)
>  #endif
>  
> +#ifndef rlimit_as_extra
> +#define rlimit_as_extra() 0
> +#endif

And this?

>  #endif /* _UAPI_ASM_GENERIC_RESOURCE_H */
> diff --git a/mm/mmap.c b/mm/mmap.c
> index fa581ced3f56..397b8cb0b0af 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -3237,7 +3237,17 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
>   */
>  bool may_expand_vm(struct mm_struct *mm, vm_flags_t flags, unsigned long npages)
>  {
> -	if (mm->total_vm + npages > rlimit(RLIMIT_AS) >> PAGE_SHIFT)
> +	unsigned long as_limit = rlimit(RLIMIT_AS);
> +	unsigned long as_limit_plus = as_limit + rlimit_as_extra();
> +
> +	/* as_limit_plus overflowed */
> +	if (as_limit_plus < as_limit)
> +		as_limit_plus = RLIM_INFINITY;
> +
> +	if (as_limit_plus > as_limit)
> +		as_limit = as_limit_plus;
> +
> +	if (mm->total_vm + npages > as_limit >> PAGE_SHIFT)

I wonder, how realistic a scenario where a userspace application enables IBT,
configures a huge prefetchable IO memory region (that just ignores bits
of offset beyond 16, for example), and start repeatedly loading a legacy
library there at different linear addresses.

>  		return false;
>  
>  	if (is_data_mapping(flags) &&
> -- 
> 2.17.1
> 

Re: [RFC PATCH v4 4/9] mm/mmap: Add IBT bitmap size to address space limit check

[Eugene Syromiatnikov]

On Fri, Sep 21, 2018 at 08:05:48AM -0700, Yu-cheng Yu wrote:
> The indirect branch tracking legacy bitmap takes a large address
> space.  This causes may_expand_vm() failure on the address limit
> check.  For a IBT-enabled task, add the bitmap size to the
> address limit.
> 
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
>  arch/x86/include/uapi/asm/resource.h |  5 +++++
>  include/uapi/asm-generic/resource.h  |  3 +++
>  mm/mmap.c                            | 12 +++++++++++-
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/uapi/asm/resource.h b/arch/x86/include/uapi/asm/resource.h
> index 04bc4db8921b..0741b2a6101a 100644
> --- a/arch/x86/include/uapi/asm/resource.h
> +++ b/arch/x86/include/uapi/asm/resource.h
> @@ -1 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> +#ifdef CONFIG_X86_INTEL_CET
> +#define rlimit_as_extra() current->thread.cet.ibt_bitmap_size
> +#endif

Does this really belong to UAPI?

> +
>  #include <asm-generic/resource.h>
> diff --git a/include/uapi/asm-generic/resource.h b/include/uapi/asm-generic/resource.h
> index f12db7a0da64..8a7608a09700 100644
> --- a/include/uapi/asm-generic/resource.h
> +++ b/include/uapi/asm-generic/resource.h
> @@ -58,5 +58,8 @@
>  # define RLIM_INFINITY		(~0UL)
>  #endif
>  
> +#ifndef rlimit_as_extra
> +#define rlimit_as_extra() 0
> +#endif

And this?

>  #endif /* _UAPI_ASM_GENERIC_RESOURCE_H */
> diff --git a/mm/mmap.c b/mm/mmap.c
> index fa581ced3f56..397b8cb0b0af 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -3237,7 +3237,17 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
>   */
>  bool may_expand_vm(struct mm_struct *mm, vm_flags_t flags, unsigned long npages)
>  {
> -	if (mm->total_vm + npages > rlimit(RLIMIT_AS) >> PAGE_SHIFT)
> +	unsigned long as_limit = rlimit(RLIMIT_AS);
> +	unsigned long as_limit_plus = as_limit + rlimit_as_extra();
> +
> +	/* as_limit_plus overflowed */
> +	if (as_limit_plus < as_limit)
> +		as_limit_plus = RLIM_INFINITY;
> +
> +	if (as_limit_plus > as_limit)
> +		as_limit = as_limit_plus;
> +
> +	if (mm->total_vm + npages > as_limit >> PAGE_SHIFT)

I wonder, how realistic a scenario where a userspace application enables IBT,
configures a huge prefetchable IO memory region (that just ignores bits
of offset beyond 16, for example), and start repeatedly loading a legacy
library there at different linear addresses.

>  		return false;
>  
>  	if (is_data_mapping(flags) &&
> -- 
> 2.17.1
> 

[RFC PATCH v4 5/9] x86/cet/ibt: ELF header parsing for IBT

[Yu-cheng Yu]

Look in .note.gnu.property of an ELF file and check if Indirect
Branch Tracking needs to be enabled for the task.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/include/uapi/asm/elf_property.h | 1 +
 arch/x86/kernel/elf.c                    | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/elf_property.h b/arch/x86/include/uapi/asm/elf_property.h
index af361207718c..343a871b8fc1 100644
--- a/arch/x86/include/uapi/asm/elf_property.h
+++ b/arch/x86/include/uapi/asm/elf_property.h
@@ -11,5 +11,6 @@
  * Bits for GNU_PROPERTY_X86_FEATURE_1_AND
  */
 #define GNU_PROPERTY_X86_FEATURE_1_SHSTK	(0x00000002)
+#define GNU_PROPERTY_X86_FEATURE_1_IBT		(0x00000001)
 
 #endif /* _UAPI_ASM_X86_ELF_PROPERTY_H */
diff --git a/arch/x86/kernel/elf.c b/arch/x86/kernel/elf.c
index 2fddd0bc545b..13358026cd64 100644
--- a/arch/x86/kernel/elf.c
+++ b/arch/x86/kernel/elf.c
@@ -300,7 +300,8 @@ int arch_setup_features(void *ehdr_p, void *phdr_p,
 
 	struct elf64_hdr *ehdr64 = ehdr_p;
 
-	if (!cpu_feature_enabled(X86_FEATURE_SHSTK))
+	if (!cpu_feature_enabled(X86_FEATURE_SHSTK) &&
+	    !cpu_feature_enabled(X86_FEATURE_IBT))
 		return 0;
 
 	if (ehdr64->e_ident[EI_CLASS] == ELFCLASS64) {
@@ -335,6 +336,11 @@ int arch_setup_features(void *ehdr_p, void *phdr_p,
 		}
 	}
 
+	if (cpu_feature_enabled(X86_FEATURE_IBT)) {
+		if (feature & GNU_PROPERTY_X86_FEATURE_1_IBT)
+			err = cet_setup_ibt();
+	}
+
 out:
 	return err;
 }
-- 
2.17.1

[RFC PATCH v4 5/9] x86/cet/ibt: ELF header parsing for IBT

[Yu-cheng Yu]

Look in .note.gnu.property of an ELF file and check if Indirect
Branch Tracking needs to be enabled for the task.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/include/uapi/asm/elf_property.h | 1 +
 arch/x86/kernel/elf.c                    | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/elf_property.h b/arch/x86/include/uapi/asm/elf_property.h
index af361207718c..343a871b8fc1 100644
--- a/arch/x86/include/uapi/asm/elf_property.h
+++ b/arch/x86/include/uapi/asm/elf_property.h
@@ -11,5 +11,6 @@
  * Bits for GNU_PROPERTY_X86_FEATURE_1_AND
  */
 #define GNU_PROPERTY_X86_FEATURE_1_SHSTK	(0x00000002)
+#define GNU_PROPERTY_X86_FEATURE_1_IBT		(0x00000001)
 
 #endif /* _UAPI_ASM_X86_ELF_PROPERTY_H */
diff --git a/arch/x86/kernel/elf.c b/arch/x86/kernel/elf.c
index 2fddd0bc545b..13358026cd64 100644
--- a/arch/x86/kernel/elf.c
+++ b/arch/x86/kernel/elf.c
@@ -300,7 +300,8 @@ int arch_setup_features(void *ehdr_p, void *phdr_p,
 
 	struct elf64_hdr *ehdr64 = ehdr_p;
 
-	if (!cpu_feature_enabled(X86_FEATURE_SHSTK))
+	if (!cpu_feature_enabled(X86_FEATURE_SHSTK) &&
+	    !cpu_feature_enabled(X86_FEATURE_IBT))
 		return 0;
 
 	if (ehdr64->e_ident[EI_CLASS] == ELFCLASS64) {
@@ -335,6 +336,11 @@ int arch_setup_features(void *ehdr_p, void *phdr_p,
 		}
 	}
 
+	if (cpu_feature_enabled(X86_FEATURE_IBT)) {
+		if (feature & GNU_PROPERTY_X86_FEATURE_1_IBT)
+			err = cet_setup_ibt();
+	}
+
 out:
 	return err;
 }
-- 
2.17.1

[RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Yu-cheng Yu]

Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
Branch Tracking features.

Introduce:

arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
    Enable the Indirect Branch Tracking legacy code bitmap.

    The parameter 'addr' is a pointer to a user buffer.
    On returning to the caller, the kernel fills the following:

    *addr = IBT bitmap base address
    *(addr + 1) = IBT bitmap size

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/include/uapi/asm/prctl.h |  1 +
 arch/x86/kernel/cet_prctl.c       | 38 ++++++++++++++++++++++++++++++-
 arch/x86/kernel/process.c         |  1 +
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h
index 3aec1088e01d..31d2465f9caf 100644
--- a/arch/x86/include/uapi/asm/prctl.h
+++ b/arch/x86/include/uapi/asm/prctl.h
@@ -18,5 +18,6 @@
 #define ARCH_CET_DISABLE	0x3002
 #define ARCH_CET_LOCK		0x3003
 #define ARCH_CET_ALLOC_SHSTK	0x3004
+#define ARCH_CET_LEGACY_BITMAP	0x3005
 
 #endif /* _ASM_X86_PRCTL_H */
diff --git a/arch/x86/kernel/cet_prctl.c b/arch/x86/kernel/cet_prctl.c
index c4b7c19f5040..df47b5ebc3f4 100644
--- a/arch/x86/kernel/cet_prctl.c
+++ b/arch/x86/kernel/cet_prctl.c
@@ -20,6 +20,8 @@ static int handle_get_status(unsigned long arg2)
 
 	if (current->thread.cet.shstk_enabled)
 		features |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+	if (current->thread.cet.ibt_enabled)
+		features |= GNU_PROPERTY_X86_FEATURE_1_IBT;
 
 	shstk_base = current->thread.cet.shstk_base;
 	shstk_size = current->thread.cet.shstk_size;
@@ -49,9 +51,35 @@ static int handle_alloc_shstk(unsigned long arg2)
 	return 0;
 }
 
+static int handle_bitmap(unsigned long arg2)
+{
+	unsigned long addr, size;
+
+	if (current->thread.cet.ibt_enabled) {
+		int err;
+
+		err  = cet_setup_ibt_bitmap();
+		if (err)
+			return err;
+
+		addr = current->thread.cet.ibt_bitmap_addr;
+		size = current->thread.cet.ibt_bitmap_size;
+	} else {
+		addr = 0;
+		size = 0;
+	}
+
+	if (put_user(addr, (unsigned long __user *)arg2) ||
+	    put_user(size, (unsigned long __user *)arg2 + 1))
+		return -EFAULT;
+
+	return 0;
+}
+
 int prctl_cet(int option, unsigned long arg2)
 {
-	if (!cpu_feature_enabled(X86_FEATURE_SHSTK))
+	if (!cpu_feature_enabled(X86_FEATURE_SHSTK) &&
+	    !cpu_feature_enabled(X86_FEATURE_IBT))
 		return -EINVAL;
 
 	switch (option) {
@@ -63,6 +91,8 @@ int prctl_cet(int option, unsigned long arg2)
 			return -EPERM;
 		if (arg2 & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
 			cet_disable_free_shstk(current);
+		if (arg2 & GNU_PROPERTY_X86_FEATURE_1_IBT)
+			cet_disable_ibt();
 
 		return 0;
 
@@ -73,6 +103,12 @@ int prctl_cet(int option, unsigned long arg2)
 	case ARCH_CET_ALLOC_SHSTK:
 		return handle_alloc_shstk(arg2);
 
+	/*
+	 * Allocate legacy bitmap and return address & size to user.
+	 */
+	case ARCH_CET_LEGACY_BITMAP:
+		return handle_bitmap(arg2);
+
 	default:
 		return -EINVAL;
 	}
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index ac0ea9c7e89f..aea15a9b6a3e 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -797,6 +797,7 @@ long do_arch_prctl_common(struct task_struct *task, int option,
 	case ARCH_CET_DISABLE:
 	case ARCH_CET_LOCK:
 	case ARCH_CET_ALLOC_SHSTK:
+	case ARCH_CET_LEGACY_BITMAP:
 		return prctl_cet(option, cpuid_enabled);
 	}
 
-- 
2.17.1

[RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Yu-cheng Yu]

Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
Branch Tracking features.

Introduce:

arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
    Enable the Indirect Branch Tracking legacy code bitmap.

    The parameter 'addr' is a pointer to a user buffer.
    On returning to the caller, the kernel fills the following:

    *addr = IBT bitmap base address
    *(addr + 1) = IBT bitmap size

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/include/uapi/asm/prctl.h |  1 +
 arch/x86/kernel/cet_prctl.c       | 38 ++++++++++++++++++++++++++++++-
 arch/x86/kernel/process.c         |  1 +
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h
index 3aec1088e01d..31d2465f9caf 100644
--- a/arch/x86/include/uapi/asm/prctl.h
+++ b/arch/x86/include/uapi/asm/prctl.h
@@ -18,5 +18,6 @@
 #define ARCH_CET_DISABLE	0x3002
 #define ARCH_CET_LOCK		0x3003
 #define ARCH_CET_ALLOC_SHSTK	0x3004
+#define ARCH_CET_LEGACY_BITMAP	0x3005
 
 #endif /* _ASM_X86_PRCTL_H */
diff --git a/arch/x86/kernel/cet_prctl.c b/arch/x86/kernel/cet_prctl.c
index c4b7c19f5040..df47b5ebc3f4 100644
--- a/arch/x86/kernel/cet_prctl.c
+++ b/arch/x86/kernel/cet_prctl.c
@@ -20,6 +20,8 @@ static int handle_get_status(unsigned long arg2)
 
 	if (current->thread.cet.shstk_enabled)
 		features |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+	if (current->thread.cet.ibt_enabled)
+		features |= GNU_PROPERTY_X86_FEATURE_1_IBT;
 
 	shstk_base = current->thread.cet.shstk_base;
 	shstk_size = current->thread.cet.shstk_size;
@@ -49,9 +51,35 @@ static int handle_alloc_shstk(unsigned long arg2)
 	return 0;
 }
 
+static int handle_bitmap(unsigned long arg2)
+{
+	unsigned long addr, size;
+
+	if (current->thread.cet.ibt_enabled) {
+		int err;
+
+		err  = cet_setup_ibt_bitmap();
+		if (err)
+			return err;
+
+		addr = current->thread.cet.ibt_bitmap_addr;
+		size = current->thread.cet.ibt_bitmap_size;
+	} else {
+		addr = 0;
+		size = 0;
+	}
+
+	if (put_user(addr, (unsigned long __user *)arg2) ||
+	    put_user(size, (unsigned long __user *)arg2 + 1))
+		return -EFAULT;
+
+	return 0;
+}
+
 int prctl_cet(int option, unsigned long arg2)
 {
-	if (!cpu_feature_enabled(X86_FEATURE_SHSTK))
+	if (!cpu_feature_enabled(X86_FEATURE_SHSTK) &&
+	    !cpu_feature_enabled(X86_FEATURE_IBT))
 		return -EINVAL;
 
 	switch (option) {
@@ -63,6 +91,8 @@ int prctl_cet(int option, unsigned long arg2)
 			return -EPERM;
 		if (arg2 & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
 			cet_disable_free_shstk(current);
+		if (arg2 & GNU_PROPERTY_X86_FEATURE_1_IBT)
+			cet_disable_ibt();
 
 		return 0;
 
@@ -73,6 +103,12 @@ int prctl_cet(int option, unsigned long arg2)
 	case ARCH_CET_ALLOC_SHSTK:
 		return handle_alloc_shstk(arg2);
 
+	/*
+	 * Allocate legacy bitmap and return address & size to user.
+	 */
+	case ARCH_CET_LEGACY_BITMAP:
+		return handle_bitmap(arg2);
+
 	default:
 		return -EINVAL;
 	}
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index ac0ea9c7e89f..aea15a9b6a3e 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -797,6 +797,7 @@ long do_arch_prctl_common(struct task_struct *task, int option,
 	case ARCH_CET_DISABLE:
 	case ARCH_CET_LOCK:
 	case ARCH_CET_ALLOC_SHSTK:
+	case ARCH_CET_LEGACY_BITMAP:
 		return prctl_cet(option, cpuid_enabled);
 	}
 
-- 
2.17.1

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Eugene Syromiatnikov]

On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
> Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
> Branch Tracking features.
> 
> Introduce:
> 
> arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
>     Enable the Indirect Branch Tracking legacy code bitmap.
> 
>     The parameter 'addr' is a pointer to a user buffer.
>     On returning to the caller, the kernel fills the following:
> 
>     *addr = IBT bitmap base address
>     *(addr + 1) = IBT bitmap size

Again, some structure with a size field would be better from
UAPI/extensibility standpoint.

One additional point: "size" in the structure from kernel should have
structure size expected by kernel, and at least providing there "0" from
user space shouldn't lead to failure (in fact, it is possible to provide
structure size back to userspace even if buffer is too small, along
with error).

> 
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
>  arch/x86/include/uapi/asm/prctl.h |  1 +
>  arch/x86/kernel/cet_prctl.c       | 38 ++++++++++++++++++++++++++++++-
>  arch/x86/kernel/process.c         |  1 +
>  3 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h
> index 3aec1088e01d..31d2465f9caf 100644
> --- a/arch/x86/include/uapi/asm/prctl.h
> +++ b/arch/x86/include/uapi/asm/prctl.h
> @@ -18,5 +18,6 @@
>  #define ARCH_CET_DISABLE	0x3002
>  #define ARCH_CET_LOCK		0x3003
>  #define ARCH_CET_ALLOC_SHSTK	0x3004
> +#define ARCH_CET_LEGACY_BITMAP	0x3005

It would probably be nice to have mention of an architecture in these
definitions ("ARCH_X86_CET_"...), but it's likely too late.

>  
>  #endif /* _ASM_X86_PRCTL_H */
> diff --git a/arch/x86/kernel/cet_prctl.c b/arch/x86/kernel/cet_prctl.c
> index c4b7c19f5040..df47b5ebc3f4 100644
> --- a/arch/x86/kernel/cet_prctl.c
> +++ b/arch/x86/kernel/cet_prctl.c
> @@ -20,6 +20,8 @@ static int handle_get_status(unsigned long arg2)
>  
>  	if (current->thread.cet.shstk_enabled)
>  		features |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
> +	if (current->thread.cet.ibt_enabled)
> +		features |= GNU_PROPERTY_X86_FEATURE_1_IBT;
>  
>  	shstk_base = current->thread.cet.shstk_base;
>  	shstk_size = current->thread.cet.shstk_size;
> @@ -49,9 +51,35 @@ static int handle_alloc_shstk(unsigned long arg2)
>  	return 0;
>  }
>  
> +static int handle_bitmap(unsigned long arg2)
> +{
> +	unsigned long addr, size;
> +
> +	if (current->thread.cet.ibt_enabled) {
> +		int err;
> +
> +		err  = cet_setup_ibt_bitmap();
> +		if (err)
> +			return err;
> +
> +		addr = current->thread.cet.ibt_bitmap_addr;
> +		size = current->thread.cet.ibt_bitmap_size;
> +	} else {
> +		addr = 0;
> +		size = 0;
> +	}
> +
> +	if (put_user(addr, (unsigned long __user *)arg2) ||
> +	    put_user(size, (unsigned long __user *)arg2 + 1))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
>  int prctl_cet(int option, unsigned long arg2)
>  {
> -	if (!cpu_feature_enabled(X86_FEATURE_SHSTK))
> +	if (!cpu_feature_enabled(X86_FEATURE_SHSTK) &&
> +	    !cpu_feature_enabled(X86_FEATURE_IBT))

This check is repeated many times, it is probably worth defining
something like cpu_x86_cet_enabled() or something like that.
Besides, early introduction of the macro would allow avoiding all these
changes over the code in IBT patches, only macro definition has
to be changed that way.

> @@ -73,6 +103,12 @@ int prctl_cet(int option, unsigned long arg2)
>  	case ARCH_CET_ALLOC_SHSTK:
>  		return handle_alloc_shstk(arg2);
>  
> +	/*
> +	 * Allocate legacy bitmap and return address & size to user.
> +	 */
> +	case ARCH_CET_LEGACY_BITMAP:
> +		return handle_bitmap(arg2);
> +
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index ac0ea9c7e89f..aea15a9b6a3e 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -797,6 +797,7 @@ long do_arch_prctl_common(struct task_struct *task, int option,
>  	case ARCH_CET_DISABLE:
>  	case ARCH_CET_LOCK:
>  	case ARCH_CET_ALLOC_SHSTK:
> +	case ARCH_CET_LEGACY_BITMAP:
>  		return prctl_cet(option, cpuid_enabled);
>  	}

I wonder, whether this duplication is really needed for CET-related
arch_prctl commands, why not just call them from do_arch_prctl_common?

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Eugene Syromiatnikov]

On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
> Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
> Branch Tracking features.
> 
> Introduce:
> 
> arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
>     Enable the Indirect Branch Tracking legacy code bitmap.
> 
>     The parameter 'addr' is a pointer to a user buffer.
>     On returning to the caller, the kernel fills the following:
> 
>     *addr = IBT bitmap base address
>     *(addr + 1) = IBT bitmap size

Again, some structure with a size field would be better from
UAPI/extensibility standpoint.

One additional point: "size" in the structure from kernel should have
structure size expected by kernel, and at least providing there "0" from
user space shouldn't lead to failure (in fact, it is possible to provide
structure size back to userspace even if buffer is too small, along
with error).

> 
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
>  arch/x86/include/uapi/asm/prctl.h |  1 +
>  arch/x86/kernel/cet_prctl.c       | 38 ++++++++++++++++++++++++++++++-
>  arch/x86/kernel/process.c         |  1 +
>  3 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h
> index 3aec1088e01d..31d2465f9caf 100644
> --- a/arch/x86/include/uapi/asm/prctl.h
> +++ b/arch/x86/include/uapi/asm/prctl.h
> @@ -18,5 +18,6 @@
>  #define ARCH_CET_DISABLE	0x3002
>  #define ARCH_CET_LOCK		0x3003
>  #define ARCH_CET_ALLOC_SHSTK	0x3004
> +#define ARCH_CET_LEGACY_BITMAP	0x3005

It would probably be nice to have mention of an architecture in these
definitions ("ARCH_X86_CET_"...), but it's likely too late.

>  
>  #endif /* _ASM_X86_PRCTL_H */
> diff --git a/arch/x86/kernel/cet_prctl.c b/arch/x86/kernel/cet_prctl.c
> index c4b7c19f5040..df47b5ebc3f4 100644
> --- a/arch/x86/kernel/cet_prctl.c
> +++ b/arch/x86/kernel/cet_prctl.c
> @@ -20,6 +20,8 @@ static int handle_get_status(unsigned long arg2)
>  
>  	if (current->thread.cet.shstk_enabled)
>  		features |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
> +	if (current->thread.cet.ibt_enabled)
> +		features |= GNU_PROPERTY_X86_FEATURE_1_IBT;
>  
>  	shstk_base = current->thread.cet.shstk_base;
>  	shstk_size = current->thread.cet.shstk_size;
> @@ -49,9 +51,35 @@ static int handle_alloc_shstk(unsigned long arg2)
>  	return 0;
>  }
>  
> +static int handle_bitmap(unsigned long arg2)
> +{
> +	unsigned long addr, size;
> +
> +	if (current->thread.cet.ibt_enabled) {
> +		int err;
> +
> +		err  = cet_setup_ibt_bitmap();
> +		if (err)
> +			return err;
> +
> +		addr = current->thread.cet.ibt_bitmap_addr;
> +		size = current->thread.cet.ibt_bitmap_size;
> +	} else {
> +		addr = 0;
> +		size = 0;
> +	}
> +
> +	if (put_user(addr, (unsigned long __user *)arg2) ||
> +	    put_user(size, (unsigned long __user *)arg2 + 1))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
>  int prctl_cet(int option, unsigned long arg2)
>  {
> -	if (!cpu_feature_enabled(X86_FEATURE_SHSTK))
> +	if (!cpu_feature_enabled(X86_FEATURE_SHSTK) &&
> +	    !cpu_feature_enabled(X86_FEATURE_IBT))

This check is repeated many times, it is probably worth defining
something like cpu_x86_cet_enabled() or something like that.
Besides, early introduction of the macro would allow avoiding all these
changes over the code in IBT patches, only macro definition has
to be changed that way.

> @@ -73,6 +103,12 @@ int prctl_cet(int option, unsigned long arg2)
>  	case ARCH_CET_ALLOC_SHSTK:
>  		return handle_alloc_shstk(arg2);
>  
> +	/*
> +	 * Allocate legacy bitmap and return address & size to user.
> +	 */
> +	case ARCH_CET_LEGACY_BITMAP:
> +		return handle_bitmap(arg2);
> +
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index ac0ea9c7e89f..aea15a9b6a3e 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -797,6 +797,7 @@ long do_arch_prctl_common(struct task_struct *task, int option,
>  	case ARCH_CET_DISABLE:
>  	case ARCH_CET_LOCK:
>  	case ARCH_CET_ALLOC_SHSTK:
> +	case ARCH_CET_LEGACY_BITMAP:
>  		return prctl_cet(option, cpuid_enabled);
>  	}

I wonder, whether this duplication is really needed for CET-related
arch_prctl commands, why not just call them from do_arch_prctl_common?

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Yu-cheng Yu]

On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
> On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
> > Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
> > Branch Tracking features.
> > 
> > Introduce:
> > 
> > arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
> >     Enable the Indirect Branch Tracking legacy code bitmap.
> > 
> >     The parameter 'addr' is a pointer to a user buffer.
> >     On returning to the caller, the kernel fills the following:
> > 
> >     *addr = IBT bitmap base address
> >     *(addr + 1) = IBT bitmap size
> 
> Again, some structure with a size field would be better from
> UAPI/extensibility standpoint.
> 
> One additional point: "size" in the structure from kernel should have
> structure size expected by kernel, and at least providing there "0" from
> user space shouldn't lead to failure (in fact, it is possible to provide
> structure size back to userspace even if buffer is too small, along
> with error).

This has been in GLIBC v2.28.  We cannot change it anymore.

> 
> > 
> > Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > ---
> >  arch/x86/include/uapi/asm/prctl.h |  1 +
> >  arch/x86/kernel/cet_prctl.c       | 38 ++++++++++++++++++++++++++++++-
> >  arch/x86/kernel/process.c         |  1 +
> >  3 files changed, 39 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/uapi/asm/prctl.h
> > b/arch/x86/include/uapi/asm/prctl.h
> > index 3aec1088e01d..31d2465f9caf 100644
> > --- a/arch/x86/include/uapi/asm/prctl.h
> > +++ b/arch/x86/include/uapi/asm/prctl.h
> > @@ -18,5 +18,6 @@
> >  #define ARCH_CET_DISABLE	0x3002
> >  #define ARCH_CET_LOCK		0x3003
> >  #define ARCH_CET_ALLOC_SHSTK	0x3004
> > +#define ARCH_CET_LEGACY_BITMAP	0x3005
> 
> It would probably be nice to have mention of an architecture in these
> definitions ("ARCH_X86_CET_"...), but it's likely too late.

We can still change macro names.  I will work on that.

> 
> >  
> >  #endif /* _ASM_X86_PRCTL_H */
> > diff --git a/arch/x86/kernel/cet_prctl.c b/arch/x86/kernel/cet_prctl.c
> > index c4b7c19f5040..df47b5ebc3f4 100644
> > --- a/arch/x86/kernel/cet_prctl.c
> > +++ b/arch/x86/kernel/cet_prctl.c
> > @@ -20,6 +20,8 @@ static int handle_get_status(unsigned long arg2)
> >  
> >  	if (current->thread.cet.shstk_enabled)
> >  		features |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
> > +	if (current->thread.cet.ibt_enabled)
> > +		features |= GNU_PROPERTY_X86_FEATURE_1_IBT;
> >  
> >  	shstk_base = current->thread.cet.shstk_base;
> >  	shstk_size = current->thread.cet.shstk_size;
> > @@ -49,9 +51,35 @@ static int handle_alloc_shstk(unsigned long arg2)
> >  	return 0;
> >  }
> >  
> > +static int handle_bitmap(unsigned long arg2)
> > +{
> > +	unsigned long addr, size;
> > +
> > +	if (current->thread.cet.ibt_enabled) {
> > +		int err;
> > +
> > +		err  = cet_setup_ibt_bitmap();
> > +		if (err)
> > +			return err;
> > +
> > +		addr = current->thread.cet.ibt_bitmap_addr;
> > +		size = current->thread.cet.ibt_bitmap_size;
> > +	} else {
> > +		addr = 0;
> > +		size = 0;
> > +	}
> > +
> > +	if (put_user(addr, (unsigned long __user *)arg2) ||
> > +	    put_user(size, (unsigned long __user *)arg2 + 1))
> > +		return -EFAULT;
> > +
> > +	return 0;
> > +}
> > +
> >  int prctl_cet(int option, unsigned long arg2)
> >  {
> > -	if (!cpu_feature_enabled(X86_FEATURE_SHSTK))
> > +	if (!cpu_feature_enabled(X86_FEATURE_SHSTK) &&
> > +	    !cpu_feature_enabled(X86_FEATURE_IBT))
> 
> This check is repeated many times, it is probably worth defining
> something like cpu_x86_cet_enabled() or something like that.
> Besides, early introduction of the macro would allow avoiding all these
> changes over the code in IBT patches, only macro definition has
> to be changed that way.

Yes, that makes things easier.

> 
> > @@ -73,6 +103,12 @@ int prctl_cet(int option, unsigned long arg2)
> >  	case ARCH_CET_ALLOC_SHSTK:
> >  		return handle_alloc_shstk(arg2);
> >  
> > +	/*
> > +	 * Allocate legacy bitmap and return address & size to user.
> > +	 */
> > +	case ARCH_CET_LEGACY_BITMAP:
> > +		return handle_bitmap(arg2);
> > +
> >  	default:
> >  		return -EINVAL;
> >  	}
> > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> > index ac0ea9c7e89f..aea15a9b6a3e 100644
> > --- a/arch/x86/kernel/process.c
> > +++ b/arch/x86/kernel/process.c
> > @@ -797,6 +797,7 @@ long do_arch_prctl_common(struct task_struct *task, int
> > option,
> >  	case ARCH_CET_DISABLE:
> >  	case ARCH_CET_LOCK:
> >  	case ARCH_CET_ALLOC_SHSTK:
> > +	case ARCH_CET_LEGACY_BITMAP:
> >  		return prctl_cet(option, cpuid_enabled);
> >  	}
> 
> I wonder, whether this duplication is really needed for CET-related
> arch_prctl commands, why not just call them from do_arch_prctl_common?

I will fix it.

Yu-cheng

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Yu-cheng Yu]

On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
> On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
> > Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
> > Branch Tracking features.
> > 
> > Introduce:
> > 
> > arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
> >     Enable the Indirect Branch Tracking legacy code bitmap.
> > 
> >     The parameter 'addr' is a pointer to a user buffer.
> >     On returning to the caller, the kernel fills the following:
> > 
> >     *addr = IBT bitmap base address
> >     *(addr + 1) = IBT bitmap size
> 
> Again, some structure with a size field would be better from
> UAPI/extensibility standpoint.
> 
> One additional point: "size" in the structure from kernel should have
> structure size expected by kernel, and at least providing there "0" from
> user space shouldn't lead to failure (in fact, it is possible to provide
> structure size back to userspace even if buffer is too small, along
> with error).

This has been in GLIBC v2.28.  We cannot change it anymore.

> 
> > 
> > Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > ---
> >  arch/x86/include/uapi/asm/prctl.h |  1 +
> >  arch/x86/kernel/cet_prctl.c       | 38 ++++++++++++++++++++++++++++++-
> >  arch/x86/kernel/process.c         |  1 +
> >  3 files changed, 39 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/uapi/asm/prctl.h
> > b/arch/x86/include/uapi/asm/prctl.h
> > index 3aec1088e01d..31d2465f9caf 100644
> > --- a/arch/x86/include/uapi/asm/prctl.h
> > +++ b/arch/x86/include/uapi/asm/prctl.h
> > @@ -18,5 +18,6 @@
> >  #define ARCH_CET_DISABLE	0x3002
> >  #define ARCH_CET_LOCK		0x3003
> >  #define ARCH_CET_ALLOC_SHSTK	0x3004
> > +#define ARCH_CET_LEGACY_BITMAP	0x3005
> 
> It would probably be nice to have mention of an architecture in these
> definitions ("ARCH_X86_CET_"...), but it's likely too late.

We can still change macro names.  I will work on that.

> 
> >  
> >  #endif /* _ASM_X86_PRCTL_H */
> > diff --git a/arch/x86/kernel/cet_prctl.c b/arch/x86/kernel/cet_prctl.c
> > index c4b7c19f5040..df47b5ebc3f4 100644
> > --- a/arch/x86/kernel/cet_prctl.c
> > +++ b/arch/x86/kernel/cet_prctl.c
> > @@ -20,6 +20,8 @@ static int handle_get_status(unsigned long arg2)
> >  
> >  	if (current->thread.cet.shstk_enabled)
> >  		features |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
> > +	if (current->thread.cet.ibt_enabled)
> > +		features |= GNU_PROPERTY_X86_FEATURE_1_IBT;
> >  
> >  	shstk_base = current->thread.cet.shstk_base;
> >  	shstk_size = current->thread.cet.shstk_size;
> > @@ -49,9 +51,35 @@ static int handle_alloc_shstk(unsigned long arg2)
> >  	return 0;
> >  }
> >  
> > +static int handle_bitmap(unsigned long arg2)
> > +{
> > +	unsigned long addr, size;
> > +
> > +	if (current->thread.cet.ibt_enabled) {
> > +		int err;
> > +
> > +		err  = cet_setup_ibt_bitmap();
> > +		if (err)
> > +			return err;
> > +
> > +		addr = current->thread.cet.ibt_bitmap_addr;
> > +		size = current->thread.cet.ibt_bitmap_size;
> > +	} else {
> > +		addr = 0;
> > +		size = 0;
> > +	}
> > +
> > +	if (put_user(addr, (unsigned long __user *)arg2) ||
> > +	    put_user(size, (unsigned long __user *)arg2 + 1))
> > +		return -EFAULT;
> > +
> > +	return 0;
> > +}
> > +
> >  int prctl_cet(int option, unsigned long arg2)
> >  {
> > -	if (!cpu_feature_enabled(X86_FEATURE_SHSTK))
> > +	if (!cpu_feature_enabled(X86_FEATURE_SHSTK) &&
> > +	    !cpu_feature_enabled(X86_FEATURE_IBT))
> 
> This check is repeated many times, it is probably worth defining
> something like cpu_x86_cet_enabled() or something like that.
> Besides, early introduction of the macro would allow avoiding all these
> changes over the code in IBT patches, only macro definition has
> to be changed that way.

Yes, that makes things easier.

> 
> > @@ -73,6 +103,12 @@ int prctl_cet(int option, unsigned long arg2)
> >  	case ARCH_CET_ALLOC_SHSTK:
> >  		return handle_alloc_shstk(arg2);
> >  
> > +	/*
> > +	 * Allocate legacy bitmap and return address & size to user.
> > +	 */
> > +	case ARCH_CET_LEGACY_BITMAP:
> > +		return handle_bitmap(arg2);
> > +
> >  	default:
> >  		return -EINVAL;
> >  	}
> > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> > index ac0ea9c7e89f..aea15a9b6a3e 100644
> > --- a/arch/x86/kernel/process.c
> > +++ b/arch/x86/kernel/process.c
> > @@ -797,6 +797,7 @@ long do_arch_prctl_common(struct task_struct *task, int
> > option,
> >  	case ARCH_CET_DISABLE:
> >  	case ARCH_CET_LOCK:
> >  	case ARCH_CET_ALLOC_SHSTK:
> > +	case ARCH_CET_LEGACY_BITMAP:
> >  		return prctl_cet(option, cpuid_enabled);
> >  	}
> 
> I wonder, whether this duplication is really needed for CET-related
> arch_prctl commands, why not just call them from do_arch_prctl_common?

I will fix it.

Yu-cheng

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Florian Weimer]

* Yu-cheng Yu:

> On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
>> On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
>> > Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
>> > Branch Tracking features.
>> > 
>> > Introduce:
>> > 
>> > arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
>> >     Enable the Indirect Branch Tracking legacy code bitmap.
>> > 
>> >     The parameter 'addr' is a pointer to a user buffer.
>> >     On returning to the caller, the kernel fills the following:
>> > 
>> >     *addr = IBT bitmap base address
>> >     *(addr + 1) = IBT bitmap size
>> 
>> Again, some structure with a size field would be better from
>> UAPI/extensibility standpoint.
>> 
>> One additional point: "size" in the structure from kernel should have
>> structure size expected by kernel, and at least providing there "0" from
>> user space shouldn't lead to failure (in fact, it is possible to provide
>> structure size back to userspace even if buffer is too small, along
>> with error).
>
> This has been in GLIBC v2.28.  We cannot change it anymore.

In theory, you could, if you change the ARCH_CET_LEGACY_BITMAP
constant, so that glibc will not use the different arch_prctl
operation.  We could backport the change into the glibc 2.28 dynamic
linker, so that existing binaries will start using CET again.  Then
only statically linked binaries will be impacted.

It's definitely not ideal, but it's doable if the interface is
terminally broken or otherwise unacceptable.  But to me it looks like
this threshold isn't reached here.

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Florian Weimer]

* Yu-cheng Yu:

> On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
>> On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
>> > Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
>> > Branch Tracking features.
>> > 
>> > Introduce:
>> > 
>> > arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
>> >     Enable the Indirect Branch Tracking legacy code bitmap.
>> > 
>> >     The parameter 'addr' is a pointer to a user buffer.
>> >     On returning to the caller, the kernel fills the following:
>> > 
>> >     *addr = IBT bitmap base address
>> >     *(addr + 1) = IBT bitmap size
>> 
>> Again, some structure with a size field would be better from
>> UAPI/extensibility standpoint.
>> 
>> One additional point: "size" in the structure from kernel should have
>> structure size expected by kernel, and at least providing there "0" from
>> user space shouldn't lead to failure (in fact, it is possible to provide
>> structure size back to userspace even if buffer is too small, along
>> with error).
>
> This has been in GLIBC v2.28.  We cannot change it anymore.

In theory, you could, if you change the ARCH_CET_LEGACY_BITMAP
constant, so that glibc will not use the different arch_prctl
operation.  We could backport the change into the glibc 2.28 dynamic
linker, so that existing binaries will start using CET again.  Then
only statically linked binaries will be impacted.

It's definitely not ideal, but it's doable if the interface is
terminally broken or otherwise unacceptable.  But to me it looks like
this threshold isn't reached here.

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Andy Lutomirski]

On Thu, Oct 4, 2018 at 9:08 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * Yu-cheng Yu:
>
> > On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
> >> On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
> >> > Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
> >> > Branch Tracking features.
> >> >
> >> > Introduce:
> >> >
> >> > arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
> >> >     Enable the Indirect Branch Tracking legacy code bitmap.
> >> >
> >> >     The parameter 'addr' is a pointer to a user buffer.
> >> >     On returning to the caller, the kernel fills the following:
> >> >
> >> >     *addr = IBT bitmap base address
> >> >     *(addr + 1) = IBT bitmap size
> >>
> >> Again, some structure with a size field would be better from
> >> UAPI/extensibility standpoint.
> >>
> >> One additional point: "size" in the structure from kernel should have
> >> structure size expected by kernel, and at least providing there "0" from
> >> user space shouldn't lead to failure (in fact, it is possible to provide
> >> structure size back to userspace even if buffer is too small, along
> >> with error).
> >
> > This has been in GLIBC v2.28.  We cannot change it anymore.
>
> In theory, you could, if you change the ARCH_CET_LEGACY_BITMAP
> constant, so that glibc will not use the different arch_prctl
> operation.  We could backport the change into the glibc 2.28 dynamic
> linker, so that existing binaries will start using CET again.  Then
> only statically linked binaries will be impacted.
>
> It's definitely not ideal, but it's doable if the interface is
> terminally broken or otherwise unacceptable.  But to me it looks like
> this threshold isn't reached here.

I tend to agree.

But I do think there's a real problem that should be fixed and won't
affect ABI: the *name* of the prctl is pretty bad.  I read the test
several times trying to decide if you meant
ARCH_GET_CET_LEGACY_BITMAP?  But you don't.

Maybe name it ARCH_CET_CREATE_LEGACY_BITMAP?  And explicitly document
what it does if legacy bitmap already exists?

--Andy

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Andy Lutomirski]

On Thu, Oct 4, 2018 at 9:08 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * Yu-cheng Yu:
>
> > On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
> >> On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
> >> > Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
> >> > Branch Tracking features.
> >> >
> >> > Introduce:
> >> >
> >> > arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
> >> >     Enable the Indirect Branch Tracking legacy code bitmap.
> >> >
> >> >     The parameter 'addr' is a pointer to a user buffer.
> >> >     On returning to the caller, the kernel fills the following:
> >> >
> >> >     *addr = IBT bitmap base address
> >> >     *(addr + 1) = IBT bitmap size
> >>
> >> Again, some structure with a size field would be better from
> >> UAPI/extensibility standpoint.
> >>
> >> One additional point: "size" in the structure from kernel should have
> >> structure size expected by kernel, and at least providing there "0" from
> >> user space shouldn't lead to failure (in fact, it is possible to provide
> >> structure size back to userspace even if buffer is too small, along
> >> with error).
> >
> > This has been in GLIBC v2.28.  We cannot change it anymore.
>
> In theory, you could, if you change the ARCH_CET_LEGACY_BITMAP
> constant, so that glibc will not use the different arch_prctl
> operation.  We could backport the change into the glibc 2.28 dynamic
> linker, so that existing binaries will start using CET again.  Then
> only statically linked binaries will be impacted.
>
> It's definitely not ideal, but it's doable if the interface is
> terminally broken or otherwise unacceptable.  But to me it looks like
> this threshold isn't reached here.

I tend to agree.

But I do think there's a real problem that should be fixed and won't
affect ABI: the *name* of the prctl is pretty bad.  I read the test
several times trying to decide if you meant
ARCH_GET_CET_LEGACY_BITMAP?  But you don't.

Maybe name it ARCH_CET_CREATE_LEGACY_BITMAP?  And explicitly document
what it does if legacy bitmap already exists?

--Andy

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Yu-cheng Yu]

On Thu, 2018-10-04 at 09:12 -0700, Andy Lutomirski wrote:
> On Thu, Oct 4, 2018 at 9:08 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> > 
> > * Yu-cheng Yu:
> > 
> > > On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
> > > > On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
> > > > > Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
> > > > > Branch Tracking features.
> > > > > 
> > > > > Introduce:
> > > > > 
> > > > > arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
> > > > >     Enable the Indirect Branch Tracking legacy code bitmap.
> > > > > 
> > > > >     The parameter 'addr' is a pointer to a user buffer.
> > > > >     On returning to the caller, the kernel fills the following:
> > > > > 
> > > > >     *addr = IBT bitmap base address
> > > > >     *(addr + 1) = IBT bitmap size
> > > > 
> > > > Again, some structure with a size field would be better from
> > > > UAPI/extensibility standpoint.
> > > > 
> > > > One additional point: "size" in the structure from kernel should have
> > > > structure size expected by kernel, and at least providing there "0" from
> > > > user space shouldn't lead to failure (in fact, it is possible to provide
> > > > structure size back to userspace even if buffer is too small, along
> > > > with error).
> > > 
> > > This has been in GLIBC v2.28.  We cannot change it anymore.
> > 
> > In theory, you could, if you change the ARCH_CET_LEGACY_BITMAP
> > constant, so that glibc will not use the different arch_prctl
> > operation.  We could backport the change into the glibc 2.28 dynamic
> > linker, so that existing binaries will start using CET again.  Then
> > only statically linked binaries will be impacted.
> > 
> > It's definitely not ideal, but it's doable if the interface is
> > terminally broken or otherwise unacceptable.  But to me it looks like
> > this threshold isn't reached here.
> 
> I tend to agree.
> 
> But I do think there's a real problem that should be fixed and won't
> affect ABI: the *name* of the prctl is pretty bad.  I read the test
> several times trying to decide if you meant
> ARCH_GET_CET_LEGACY_BITMAP?  But you don't.
> 
> Maybe name it ARCH_CET_CREATE_LEGACY_BITMAP?  And explicitly document
> what it does if legacy bitmap already exists?

I will fix it.

Yu-cheng

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Yu-cheng Yu]

On Thu, 2018-10-04 at 09:12 -0700, Andy Lutomirski wrote:
> On Thu, Oct 4, 2018 at 9:08 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> > 
> > * Yu-cheng Yu:
> > 
> > > On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
> > > > On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
> > > > > Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
> > > > > Branch Tracking features.
> > > > > 
> > > > > Introduce:
> > > > > 
> > > > > arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
> > > > >     Enable the Indirect Branch Tracking legacy code bitmap.
> > > > > 
> > > > >     The parameter 'addr' is a pointer to a user buffer.
> > > > >     On returning to the caller, the kernel fills the following:
> > > > > 
> > > > >     *addr = IBT bitmap base address
> > > > >     *(addr + 1) = IBT bitmap size
> > > > 
> > > > Again, some structure with a size field would be better from
> > > > UAPI/extensibility standpoint.
> > > > 
> > > > One additional point: "size" in the structure from kernel should have
> > > > structure size expected by kernel, and at least providing there "0" from
> > > > user space shouldn't lead to failure (in fact, it is possible to provide
> > > > structure size back to userspace even if buffer is too small, along
> > > > with error).
> > > 
> > > This has been in GLIBC v2.28.  We cannot change it anymore.
> > 
> > In theory, you could, if you change the ARCH_CET_LEGACY_BITMAP
> > constant, so that glibc will not use the different arch_prctl
> > operation.  We could backport the change into the glibc 2.28 dynamic
> > linker, so that existing binaries will start using CET again.  Then
> > only statically linked binaries will be impacted.
> > 
> > It's definitely not ideal, but it's doable if the interface is
> > terminally broken or otherwise unacceptable.  But to me it looks like
> > this threshold isn't reached here.
> 
> I tend to agree.
> 
> But I do think there's a real problem that should be fixed and won't
> affect ABI: the *name* of the prctl is pretty bad.  I read the test
> several times trying to decide if you meant
> ARCH_GET_CET_LEGACY_BITMAP?  But you don't.
> 
> Maybe name it ARCH_CET_CREATE_LEGACY_BITMAP?  And explicitly document
> what it does if legacy bitmap already exists?

I will fix it.

Yu-cheng

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Andy Lutomirski]

> On Oct 4, 2018, at 8:37 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
>> On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
>>> On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
>>> Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
>>> Branch Tracking features.
>>>
>>> Introduce:
>>>
>>> arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
>>>    Enable the Indirect Branch Tracking legacy code bitmap.
>>>
>>>    The parameter 'addr' is a pointer to a user buffer.
>>>    On returning to the caller, the kernel fills the following:
>>>
>>>    *addr = IBT bitmap base address
>>>    *(addr + 1) = IBT bitmap size
>>
>> Again, some structure with a size field would be better from
>> UAPI/extensibility standpoint.
>>
>> One additional point: "size" in the structure from kernel should have
>> structure size expected by kernel, and at least providing there "0" from
>> user space shouldn't lead to failure (in fact, it is possible to provide
>> structure size back to userspace even if buffer is too small, along
>> with error).
>
> This has been in GLIBC v2.28.  We cannot change it anymore.

Sure you can. Just change ARCH_CET_LEGACY_BITMAP to a new number.  You
might need to change all the constants.  And if the ELF note by itself
causes a problem too, you may need to rename it.  And maybe ask glibc
to kindly not enable code that depends on non-upstreamed kernel
features.

There is not, and has never been, any ABI compatibility requirement
that says that, if glibc 2.28 "enables" a feature, that the kernel
will ever enable it in a way that makes glibc 2.28 actually support
it.  All the kernel needs to do is avoid making glibc 2.28 *crash*.

Re: [RFC PATCH v4 6/9] x86/cet/ibt: Add arch_prctl functions for IBT

[Andy Lutomirski]

> On Oct 4, 2018, at 8:37 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
>> On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
>>> On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
>>> Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
>>> Branch Tracking features.
>>>
>>> Introduce:
>>>
>>> arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
>>>    Enable the Indirect Branch Tracking legacy code bitmap.
>>>
>>>    The parameter 'addr' is a pointer to a user buffer.
>>>    On returning to the caller, the kernel fills the following:
>>>
>>>    *addr = IBT bitmap base address
>>>    *(addr + 1) = IBT bitmap size
>>
>> Again, some structure with a size field would be better from
>> UAPI/extensibility standpoint.
>>
>> One additional point: "size" in the structure from kernel should have
>> structure size expected by kernel, and at least providing there "0" from
>> user space shouldn't lead to failure (in fact, it is possible to provide
>> structure size back to userspace even if buffer is too small, along
>> with error).
>
> This has been in GLIBC v2.28.  We cannot change it anymore.

Sure you can. Just change ARCH_CET_LEGACY_BITMAP to a new number.  You
might need to change all the constants.  And if the ELF note by itself
causes a problem too, you may need to rename it.  And maybe ask glibc
to kindly not enable code that depends on non-upstreamed kernel
features.

There is not, and has never been, any ABI compatibility requirement
that says that, if glibc 2.28 "enables" a feature, that the kernel
will ever enable it in a way that makes glibc 2.28 actually support
it.  All the kernel needs to do is avoid making glibc 2.28 *crash*.

[RFC PATCH v4 7/9] x86/cet/ibt: Add ENDBR to op-code-map

[Yu-cheng Yu]

Add control transfer terminating instructions:

ENDBR64/ENDBR32:
    Mark a valid 64/32-bit control transfer endpoint.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/lib/x86-opcode-map.txt               | 13 +++++++++++--
 tools/objtool/arch/x86/lib/x86-opcode-map.txt | 13 +++++++++++--
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
index c5e825d44766..fbc53481bc59 100644
--- a/arch/x86/lib/x86-opcode-map.txt
+++ b/arch/x86/lib/x86-opcode-map.txt
@@ -620,7 +620,16 @@ ea: SAVEPREVSSP (f3)
 # Skip 0xeb-0xff
 EndTable
 
-Table: 3-byte opcode 2 (0x0f 0x38)
+Table: 3-byte opcode 2 (0x0f 0x1e)
+Referrer:
+AVXcode:
+# Skip 0x00-0xf9
+fa: ENDBR64 (f3)
+fb: ENDBR32 (f3)
+#skip 0xfc-0xff
+EndTable
+
+Table: 3-byte opcode 3 (0x0f 0x38)
 Referrer: 3-byte escape 1
 AVXcode: 2
 # 0x0f 0x38 0x00-0x0f
@@ -804,7 +813,7 @@ f6: ADCX Gy,Ey (66) | ADOX Gy,Ey (F3) | MULX By,Gy,rDX,Ey (F2),(v) | WRSS Pq,Qq
 f7: BEXTR Gy,Ey,By (v) | SHLX Gy,Ey,By (66),(v) | SARX Gy,Ey,By (F3),(v) | SHRX Gy,Ey,By (F2),(v)
 EndTable
 
-Table: 3-byte opcode 3 (0x0f 0x3a)
+Table: 3-byte opcode 4 (0x0f 0x3a)
 Referrer: 3-byte escape 2
 AVXcode: 3
 # 0x0f 0x3a 0x00-0xff
diff --git a/tools/objtool/arch/x86/lib/x86-opcode-map.txt b/tools/objtool/arch/x86/lib/x86-opcode-map.txt
index c5e825d44766..fbc53481bc59 100644
--- a/tools/objtool/arch/x86/lib/x86-opcode-map.txt
+++ b/tools/objtool/arch/x86/lib/x86-opcode-map.txt
@@ -620,7 +620,16 @@ ea: SAVEPREVSSP (f3)
 # Skip 0xeb-0xff
 EndTable
 
-Table: 3-byte opcode 2 (0x0f 0x38)
+Table: 3-byte opcode 2 (0x0f 0x1e)
+Referrer:
+AVXcode:
+# Skip 0x00-0xf9
+fa: ENDBR64 (f3)
+fb: ENDBR32 (f3)
+#skip 0xfc-0xff
+EndTable
+
+Table: 3-byte opcode 3 (0x0f 0x38)
 Referrer: 3-byte escape 1
 AVXcode: 2
 # 0x0f 0x38 0x00-0x0f
@@ -804,7 +813,7 @@ f6: ADCX Gy,Ey (66) | ADOX Gy,Ey (F3) | MULX By,Gy,rDX,Ey (F2),(v) | WRSS Pq,Qq
 f7: BEXTR Gy,Ey,By (v) | SHLX Gy,Ey,By (66),(v) | SARX Gy,Ey,By (F3),(v) | SHRX Gy,Ey,By (F2),(v)
 EndTable
 
-Table: 3-byte opcode 3 (0x0f 0x3a)
+Table: 3-byte opcode 4 (0x0f 0x3a)
 Referrer: 3-byte escape 2
 AVXcode: 3
 # 0x0f 0x3a 0x00-0xff
-- 
2.17.1

[RFC PATCH v4 7/9] x86/cet/ibt: Add ENDBR to op-code-map

[Yu-cheng Yu]

Add control transfer terminating instructions:

ENDBR64/ENDBR32:
    Mark a valid 64/32-bit control transfer endpoint.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/lib/x86-opcode-map.txt               | 13 +++++++++++--
 tools/objtool/arch/x86/lib/x86-opcode-map.txt | 13 +++++++++++--
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
index c5e825d44766..fbc53481bc59 100644
--- a/arch/x86/lib/x86-opcode-map.txt
+++ b/arch/x86/lib/x86-opcode-map.txt
@@ -620,7 +620,16 @@ ea: SAVEPREVSSP (f3)
 # Skip 0xeb-0xff
 EndTable
 
-Table: 3-byte opcode 2 (0x0f 0x38)
+Table: 3-byte opcode 2 (0x0f 0x1e)
+Referrer:
+AVXcode:
+# Skip 0x00-0xf9
+fa: ENDBR64 (f3)
+fb: ENDBR32 (f3)
+#skip 0xfc-0xff
+EndTable
+
+Table: 3-byte opcode 3 (0x0f 0x38)
 Referrer: 3-byte escape 1
 AVXcode: 2
 # 0x0f 0x38 0x00-0x0f
@@ -804,7 +813,7 @@ f6: ADCX Gy,Ey (66) | ADOX Gy,Ey (F3) | MULX By,Gy,rDX,Ey (F2),(v) | WRSS Pq,Qq
 f7: BEXTR Gy,Ey,By (v) | SHLX Gy,Ey,By (66),(v) | SARX Gy,Ey,By (F3),(v) | SHRX Gy,Ey,By (F2),(v)
 EndTable
 
-Table: 3-byte opcode 3 (0x0f 0x3a)
+Table: 3-byte opcode 4 (0x0f 0x3a)
 Referrer: 3-byte escape 2
 AVXcode: 3
 # 0x0f 0x3a 0x00-0xff
diff --git a/tools/objtool/arch/x86/lib/x86-opcode-map.txt b/tools/objtool/arch/x86/lib/x86-opcode-map.txt
index c5e825d44766..fbc53481bc59 100644
--- a/tools/objtool/arch/x86/lib/x86-opcode-map.txt
+++ b/tools/objtool/arch/x86/lib/x86-opcode-map.txt
@@ -620,7 +620,16 @@ ea: SAVEPREVSSP (f3)
 # Skip 0xeb-0xff
 EndTable
 
-Table: 3-byte opcode 2 (0x0f 0x38)
+Table: 3-byte opcode 2 (0x0f 0x1e)
+Referrer:
+AVXcode:
+# Skip 0x00-0xf9
+fa: ENDBR64 (f3)
+fb: ENDBR32 (f3)
+#skip 0xfc-0xff
+EndTable
+
+Table: 3-byte opcode 3 (0x0f 0x38)
 Referrer: 3-byte escape 1
 AVXcode: 2
 # 0x0f 0x38 0x00-0x0f
@@ -804,7 +813,7 @@ f6: ADCX Gy,Ey (66) | ADOX Gy,Ey (F3) | MULX By,Gy,rDX,Ey (F2),(v) | WRSS Pq,Qq
 f7: BEXTR Gy,Ey,By (v) | SHLX Gy,Ey,By (66),(v) | SARX Gy,Ey,By (F3),(v) | SHRX Gy,Ey,By (F2),(v)
 EndTable
 
-Table: 3-byte opcode 3 (0x0f 0x3a)
+Table: 3-byte opcode 4 (0x0f 0x3a)
 Referrer: 3-byte escape 2
 AVXcode: 3
 # 0x0f 0x3a 0x00-0xff
-- 
2.17.1

[RFC PATCH v4 8/9] x86: Insert endbr32/endbr64 to vDSO

[Yu-cheng Yu]

From: "H.J. Lu" <hjl.tools@gmail.com>

When Intel indirect branch tracking is enabled, functions in vDSO which
may be called indirectly must have endbr32 or endbr64 as the first
instruction.  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>
---
 arch/x86/entry/vdso/.gitignore        |  4 ++++
 arch/x86/entry/vdso/Makefile          | 12 +++++++++++-
 arch/x86/entry/vdso/vdso-layout.lds.S |  1 +
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/vdso/.gitignore b/arch/x86/entry/vdso/.gitignore
index aae8ffdd5880..552941fdfae0 100644
--- a/arch/x86/entry/vdso/.gitignore
+++ b/arch/x86/entry/vdso/.gitignore
@@ -5,3 +5,7 @@ vdso32-sysenter-syms.lds
 vdso32-int80-syms.lds
 vdso-image-*.c
 vdso2c
+vclock_gettime.S
+vgetcpu.S
+vclock_gettime.asm
+vgetcpu.asm
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index fa3f439f0a92..8694f70c08e6 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -102,13 +102,17 @@ vobjx32s := $(foreach F,$(vobjx32s-y),$(obj)/$F)
 
 # Convert 64bit object file to x32 for x32 vDSO.
 quiet_cmd_x32 = X32     $@
-      cmd_x32 = $(OBJCOPY) -O elf32-x86-64 $< $@
+      cmd_x32 = $(OBJCOPY) -R .note.gnu.property -O elf32-x86-64 $< $@
 
 $(obj)/%-x32.o: $(obj)/%.o FORCE
 	$(call if_changed,x32)
 
 targets += vdsox32.lds $(vobjx32s-y)
 
+ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+    $(obj)/vclock_gettime.o $(obj)/vgetcpu.o $(obj)/vdso32/vclock_gettime.o: KBUILD_CFLAGS += -fcf-protection=branch
+endif
+
 $(obj)/%.so: OBJCOPYFLAGS := -S
 $(obj)/%.so: $(obj)/%.so.dbg
 	$(call if_changed,objcopy)
@@ -160,6 +164,12 @@ quiet_cmd_vdso = VDSO    $@
 
 VDSO_LDFLAGS = -shared $(call ld-option, --hash-style=both) \
 	$(call ld-option, --build-id) -Bsymbolic
+ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+  VDSO_LDFLAGS += $(call ldoption, -z$(comma)ibt)
+endif
+ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
+  VDSO_LDFLAGS += $(call ldoption, -z$(comma)shstk)
+endif
 GCOV_PROFILE := n
 
 #
diff --git a/arch/x86/entry/vdso/vdso-layout.lds.S b/arch/x86/entry/vdso/vdso-layout.lds.S
index acfd5ba7d943..cabaeedfed78 100644
--- a/arch/x86/entry/vdso/vdso-layout.lds.S
+++ b/arch/x86/entry/vdso/vdso-layout.lds.S
@@ -74,6 +74,7 @@ SECTIONS
 	.fake_shstrtab	: { *(.fake_shstrtab) }		:text
 
 
+	.note.gnu.property : { *(.note.gnu.property) }	:text	:note
 	.note		: { *(.note.*) }		:text	:note
 
 	.eh_frame_hdr	: { *(.eh_frame_hdr) }		:text	:eh_frame_hdr
-- 
2.17.1

[RFC PATCH v4 8/9] x86: Insert endbr32/endbr64 to vDSO

[Yu-cheng Yu]

From: "H.J. Lu" <hjl.tools@gmail.com>

When Intel indirect branch tracking is enabled, functions in vDSO which
may be called indirectly must have endbr32 or endbr64 as the first
instruction.  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>
---
 arch/x86/entry/vdso/.gitignore        |  4 ++++
 arch/x86/entry/vdso/Makefile          | 12 +++++++++++-
 arch/x86/entry/vdso/vdso-layout.lds.S |  1 +
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/vdso/.gitignore b/arch/x86/entry/vdso/.gitignore
index aae8ffdd5880..552941fdfae0 100644
--- a/arch/x86/entry/vdso/.gitignore
+++ b/arch/x86/entry/vdso/.gitignore
@@ -5,3 +5,7 @@ vdso32-sysenter-syms.lds
 vdso32-int80-syms.lds
 vdso-image-*.c
 vdso2c
+vclock_gettime.S
+vgetcpu.S
+vclock_gettime.asm
+vgetcpu.asm
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index fa3f439f0a92..8694f70c08e6 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -102,13 +102,17 @@ vobjx32s := $(foreach F,$(vobjx32s-y),$(obj)/$F)
 
 # Convert 64bit object file to x32 for x32 vDSO.
 quiet_cmd_x32 = X32     $@
-      cmd_x32 = $(OBJCOPY) -O elf32-x86-64 $< $@
+      cmd_x32 = $(OBJCOPY) -R .note.gnu.property -O elf32-x86-64 $< $@
 
 $(obj)/%-x32.o: $(obj)/%.o FORCE
 	$(call if_changed,x32)
 
 targets += vdsox32.lds $(vobjx32s-y)
 
+ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+    $(obj)/vclock_gettime.o $(obj)/vgetcpu.o $(obj)/vdso32/vclock_gettime.o: KBUILD_CFLAGS += -fcf-protection=branch
+endif
+
 $(obj)/%.so: OBJCOPYFLAGS := -S
 $(obj)/%.so: $(obj)/%.so.dbg
 	$(call if_changed,objcopy)
@@ -160,6 +164,12 @@ quiet_cmd_vdso = VDSO    $@
 
 VDSO_LDFLAGS = -shared $(call ld-option, --hash-style=both) \
 	$(call ld-option, --build-id) -Bsymbolic
+ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+  VDSO_LDFLAGS += $(call ldoption, -z$(comma)ibt)
+endif
+ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
+  VDSO_LDFLAGS += $(call ldoption, -z$(comma)shstk)
+endif
 GCOV_PROFILE := n
 
 #
diff --git a/arch/x86/entry/vdso/vdso-layout.lds.S b/arch/x86/entry/vdso/vdso-layout.lds.S
index acfd5ba7d943..cabaeedfed78 100644
--- a/arch/x86/entry/vdso/vdso-layout.lds.S
+++ b/arch/x86/entry/vdso/vdso-layout.lds.S
@@ -74,6 +74,7 @@ SECTIONS
 	.fake_shstrtab	: { *(.fake_shstrtab) }		:text
 
 
+	.note.gnu.property : { *(.note.gnu.property) }	:text	:note
 	.note		: { *(.note.*) }		:text	:note
 
 	.eh_frame_hdr	: { *(.eh_frame_hdr) }		:text	:eh_frame_hdr
-- 
2.17.1

[RFC PATCH v4 9/9] x86/cet: Add PTRACE interface for CET

[Yu-cheng Yu]

Add REGSET_CET64/REGSET_CET32 to get/set CET MSRs:

    IA32_U_CET (user-mode CET settings),
    IA32_PL3_SSP (user-mode shadow stack),
    IA32_PL0_SSP (kernel-mode shadow stack),
    IA32_PL1_SSP (ring-1 shadow stack),
    IA32_PL2_SSP (ring-2 shadow stack).

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/include/asm/fpu/regset.h |  7 +++---
 arch/x86/kernel/fpu/regset.c      | 41 +++++++++++++++++++++++++++++++
 arch/x86/kernel/ptrace.c          | 16 ++++++++++++
 include/uapi/linux/elf.h          |  1 +
 4 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/fpu/regset.h b/arch/x86/include/asm/fpu/regset.h
index d5bdffb9d27f..edad0d889084 100644
--- a/arch/x86/include/asm/fpu/regset.h
+++ b/arch/x86/include/asm/fpu/regset.h
@@ -7,11 +7,12 @@
 
 #include <linux/regset.h>
 
-extern user_regset_active_fn regset_fpregs_active, regset_xregset_fpregs_active;
+extern user_regset_active_fn regset_fpregs_active, regset_xregset_fpregs_active,
+				cetregs_active;
 extern user_regset_get_fn fpregs_get, xfpregs_get, fpregs_soft_get,
-				xstateregs_get;
+				xstateregs_get, cetregs_get;
 extern user_regset_set_fn fpregs_set, xfpregs_set, fpregs_soft_set,
-				 xstateregs_set;
+				 xstateregs_set, cetregs_set;
 
 /*
  * xstateregs_active == regset_fpregs_active. Please refer to the comment
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index bc02f5144b95..7008eb084d36 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -160,6 +160,47 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 	return ret;
 }
 
+int cetregs_active(struct task_struct *target, const struct user_regset *regset)
+{
+#ifdef CONFIG_X86_INTEL_CET
+	if (target->thread.cet.shstk_enabled || target->thread.cet.ibt_enabled)
+		return regset->n;
+#endif
+	return 0;
+}
+
+int cetregs_get(struct task_struct *target, const struct user_regset *regset,
+		unsigned int pos, unsigned int count,
+		void *kbuf, void __user *ubuf)
+{
+	struct fpu *fpu = &target->thread.fpu;
+	struct cet_user_state *cetregs;
+
+	if (!boot_cpu_has(X86_FEATURE_SHSTK))
+		return -ENODEV;
+
+	cetregs = get_xsave_addr(&fpu->state.xsave, XFEATURE_MASK_SHSTK_USER);
+
+	fpu__prepare_read(fpu);
+	return user_regset_copyout(&pos, &count, &kbuf, &ubuf, cetregs, 0, -1);
+}
+
+int cetregs_set(struct task_struct *target, const struct user_regset *regset,
+		  unsigned int pos, unsigned int count,
+		  const void *kbuf, const void __user *ubuf)
+{
+	struct fpu *fpu = &target->thread.fpu;
+	struct cet_user_state *cetregs;
+
+	if (!boot_cpu_has(X86_FEATURE_SHSTK))
+		return -ENODEV;
+
+	cetregs = get_xsave_addr(&fpu->state.xsave, XFEATURE_MASK_SHSTK_USER);
+
+	fpu__prepare_write(fpu);
+	return user_regset_copyin(&pos, &count, &kbuf, &ubuf, cetregs, 0, -1);
+}
+
 #if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
 
 /*
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index e2ee403865eb..ac2bc3a18427 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -49,7 +49,9 @@ enum x86_regset {
 	REGSET_IOPERM64 = REGSET_XFP,
 	REGSET_XSTATE,
 	REGSET_TLS,
+	REGSET_CET64 = REGSET_TLS,
 	REGSET_IOPERM32,
+	REGSET_CET32,
 };
 
 struct pt_regs_offset {
@@ -1276,6 +1278,13 @@ static struct user_regset x86_64_regsets[] __ro_after_init = {
 		.size = sizeof(long), .align = sizeof(long),
 		.active = ioperm_active, .get = ioperm_get
 	},
+	[REGSET_CET64] = {
+		.core_note_type = NT_X86_CET,
+		.n = sizeof(struct cet_user_state) / sizeof(u64),
+		.size = sizeof(u64), .align = sizeof(u64),
+		.active = cetregs_active, .get = cetregs_get,
+		.set = cetregs_set
+	},
 };
 
 static const struct user_regset_view user_x86_64_view = {
@@ -1331,6 +1340,13 @@ static struct user_regset x86_32_regsets[] __ro_after_init = {
 		.size = sizeof(u32), .align = sizeof(u32),
 		.active = ioperm_active, .get = ioperm_get
 	},
+	[REGSET_CET32] = {
+		.core_note_type = NT_X86_CET,
+		.n = sizeof(struct cet_user_state) / sizeof(u64),
+		.size = sizeof(u64), .align = sizeof(u64),
+		.active = cetregs_active, .get = cetregs_get,
+		.set = cetregs_set
+	},
 };
 
 static const struct user_regset_view user_x86_32_view = {
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index 5ef25a565e88..f4cdfdc59c0a 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -401,6 +401,7 @@ typedef struct elf64_shdr {
 #define NT_386_TLS	0x200		/* i386 TLS slots (struct user_desc) */
 #define NT_386_IOPERM	0x201		/* x86 io permission bitmap (1=deny) */
 #define NT_X86_XSTATE	0x202		/* x86 extended state using xsave */
+#define NT_X86_CET	0x203		/* x86 cet state */
 #define NT_S390_HIGH_GPRS	0x300	/* s390 upper register halves */
 #define NT_S390_TIMER	0x301		/* s390 timer register */
 #define NT_S390_TODCMP	0x302		/* s390 TOD clock comparator register */
-- 
2.17.1

[RFC PATCH v4 9/9] x86/cet: Add PTRACE interface for CET

[Yu-cheng Yu]

Add REGSET_CET64/REGSET_CET32 to get/set CET MSRs:

    IA32_U_CET (user-mode CET settings),
    IA32_PL3_SSP (user-mode shadow stack),
    IA32_PL0_SSP (kernel-mode shadow stack),
    IA32_PL1_SSP (ring-1 shadow stack),
    IA32_PL2_SSP (ring-2 shadow stack).

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/include/asm/fpu/regset.h |  7 +++---
 arch/x86/kernel/fpu/regset.c      | 41 +++++++++++++++++++++++++++++++
 arch/x86/kernel/ptrace.c          | 16 ++++++++++++
 include/uapi/linux/elf.h          |  1 +
 4 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/fpu/regset.h b/arch/x86/include/asm/fpu/regset.h
index d5bdffb9d27f..edad0d889084 100644
--- a/arch/x86/include/asm/fpu/regset.h
+++ b/arch/x86/include/asm/fpu/regset.h
@@ -7,11 +7,12 @@
 
 #include <linux/regset.h>
 
-extern user_regset_active_fn regset_fpregs_active, regset_xregset_fpregs_active;
+extern user_regset_active_fn regset_fpregs_active, regset_xregset_fpregs_active,
+				cetregs_active;
 extern user_regset_get_fn fpregs_get, xfpregs_get, fpregs_soft_get,
-				xstateregs_get;
+				xstateregs_get, cetregs_get;
 extern user_regset_set_fn fpregs_set, xfpregs_set, fpregs_soft_set,
-				 xstateregs_set;
+				 xstateregs_set, cetregs_set;
 
 /*
  * xstateregs_active == regset_fpregs_active. Please refer to the comment
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index bc02f5144b95..7008eb084d36 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -160,6 +160,47 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 	return ret;
 }
 
+int cetregs_active(struct task_struct *target, const struct user_regset *regset)
+{
+#ifdef CONFIG_X86_INTEL_CET
+	if (target->thread.cet.shstk_enabled || target->thread.cet.ibt_enabled)
+		return regset->n;
+#endif
+	return 0;
+}
+
+int cetregs_get(struct task_struct *target, const struct user_regset *regset,
+		unsigned int pos, unsigned int count,
+		void *kbuf, void __user *ubuf)
+{
+	struct fpu *fpu = &target->thread.fpu;
+	struct cet_user_state *cetregs;
+
+	if (!boot_cpu_has(X86_FEATURE_SHSTK))
+		return -ENODEV;
+
+	cetregs = get_xsave_addr(&fpu->state.xsave, XFEATURE_MASK_SHSTK_USER);
+
+	fpu__prepare_read(fpu);
+	return user_regset_copyout(&pos, &count, &kbuf, &ubuf, cetregs, 0, -1);
+}
+
+int cetregs_set(struct task_struct *target, const struct user_regset *regset,
+		  unsigned int pos, unsigned int count,
+		  const void *kbuf, const void __user *ubuf)
+{
+	struct fpu *fpu = &target->thread.fpu;
+	struct cet_user_state *cetregs;
+
+	if (!boot_cpu_has(X86_FEATURE_SHSTK))
+		return -ENODEV;
+
+	cetregs = get_xsave_addr(&fpu->state.xsave, XFEATURE_MASK_SHSTK_USER);
+
+	fpu__prepare_write(fpu);
+	return user_regset_copyin(&pos, &count, &kbuf, &ubuf, cetregs, 0, -1);
+}
+
 #if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
 
 /*
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index e2ee403865eb..ac2bc3a18427 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -49,7 +49,9 @@ enum x86_regset {
 	REGSET_IOPERM64 = REGSET_XFP,
 	REGSET_XSTATE,
 	REGSET_TLS,
+	REGSET_CET64 = REGSET_TLS,
 	REGSET_IOPERM32,
+	REGSET_CET32,
 };
 
 struct pt_regs_offset {
@@ -1276,6 +1278,13 @@ static struct user_regset x86_64_regsets[] __ro_after_init = {
 		.size = sizeof(long), .align = sizeof(long),
 		.active = ioperm_active, .get = ioperm_get
 	},
+	[REGSET_CET64] = {
+		.core_note_type = NT_X86_CET,
+		.n = sizeof(struct cet_user_state) / sizeof(u64),
+		.size = sizeof(u64), .align = sizeof(u64),
+		.active = cetregs_active, .get = cetregs_get,
+		.set = cetregs_set
+	},
 };
 
 static const struct user_regset_view user_x86_64_view = {
@@ -1331,6 +1340,13 @@ static struct user_regset x86_32_regsets[] __ro_after_init = {
 		.size = sizeof(u32), .align = sizeof(u32),
 		.active = ioperm_active, .get = ioperm_get
 	},
+	[REGSET_CET32] = {
+		.core_note_type = NT_X86_CET,
+		.n = sizeof(struct cet_user_state) / sizeof(u64),
+		.size = sizeof(u64), .align = sizeof(u64),
+		.active = cetregs_active, .get = cetregs_get,
+		.set = cetregs_set
+	},
 };
 
 static const struct user_regset_view user_x86_32_view = {
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index 5ef25a565e88..f4cdfdc59c0a 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -401,6 +401,7 @@ typedef struct elf64_shdr {
 #define NT_386_TLS	0x200		/* i386 TLS slots (struct user_desc) */
 #define NT_386_IOPERM	0x201		/* x86 io permission bitmap (1=deny) */
 #define NT_X86_XSTATE	0x202		/* x86 extended state using xsave */
+#define NT_X86_CET	0x203		/* x86 cet state */
 #define NT_S390_HIGH_GPRS	0x300	/* s390 upper register halves */
 #define NT_S390_TIMER	0x301		/* s390 timer register */
 #define NT_S390_TODCMP	0x302		/* s390 TOD clock comparator register */
-- 
2.17.1