[PATCH v7 00/14] Control-flow Enforcement: Branch Tracking, PTRACE

[PATCH v7 00/14] Control-flow Enforcement: Branch Tracking, PTRACE

[Yu-cheng Yu]

The previous version of CET Branch Tracking/PTRACE patches is here:

  https://lkml.org/lkml/2018/11/20/203

Summary of changes from v6:

  Rebase to v5.2-rc3.

  Add Branch Tracking in the signal handling routines.

  Fix Branch Tracking (and Shadow Stack) for vsyscall (patch #12):
    This patch can be dropped if we expect CET blocking vsyscall.

  Include H.J. Lu's patch to discard .note.gnu.property in the kernel.

H.J. Lu (4):
  x86/vdso: Insert endbr32/endbr64 to vDSO
  x86/vdso/32: Add ENDBR32 to __kernel_vsyscall entry point
  x86/vsyscall/64: Add ENDBR64 to vsyscall entry points
  x86: Discard .note.gnu.property sections

Yu-cheng Yu (10):
  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 setup function
  x86/cet/ibt: Handle signals for IBT
  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/vsyscall/64: Fixup shadow stack and branch tracking for vsyscall
  x86/cet: Add PTRACE interface for CET

 arch/x86/Kconfig                              | 16 ++++
 arch/x86/Makefile                             |  7 ++
 arch/x86/entry/vdso/Makefile                  | 12 ++-
 arch/x86/entry/vdso/vdso-layout.lds.S         |  1 +
 arch/x86/entry/vdso/vdso32/system_call.S      |  3 +
 arch/x86/entry/vsyscall/vsyscall_64.c         | 28 +++++++
 arch/x86/entry/vsyscall/vsyscall_emu_64.S     |  9 +++
 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/asm/mmu_context.h            | 10 +++
 arch/x86/include/uapi/asm/prctl.h             |  2 +
 arch/x86/kernel/cet.c                         | 80 +++++++++++++++++++
 arch/x86/kernel/cet_prctl.c                   | 21 +++++
 arch/x86/kernel/cpu/common.c                  | 17 ++++
 arch/x86/kernel/fpu/regset.c                  | 41 ++++++++++
 arch/x86/kernel/process_64.c                  |  6 ++
 arch/x86/kernel/ptrace.c                      | 16 ++++
 arch/x86/kernel/vmlinux.lds.S                 | 11 ++-
 arch/x86/lib/x86-opcode-map.txt               | 13 ++-
 include/uapi/linux/elf.h                      |  1 +
 mm/mmap.c                                     | 19 ++++-
 .../arch/x86/include/asm/disabled-features.h  |  8 +-
 tools/objtool/arch/x86/lib/x86-opcode-map.txt | 13 ++-
 24 files changed, 345 insertions(+), 12 deletions(-)

-- 
2.17.1

[PATCH v7 01/14] 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 and feature setup.

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

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index df8b57de75b2..47afe47c01eb 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1921,6 +1921,9 @@ config X86_INTEL_CET
 config ARCH_HAS_SHSTK
 	def_bool n
 
+config ARCH_HAS_AS_LIMIT
+	def_bool n
+
 config X86_INTEL_SHADOW_STACK_USER
 	prompt "Intel Shadow Stack for user-mode"
 	def_bool n
@@ -1941,6 +1944,19 @@ 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_AS_LIMIT
+	select ARCH_USE_GNU_PROPERTY
+	---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 0b2e9df48907..25372cc4a303 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -155,6 +155,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

[PATCH v7 02/14] 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                    |  7 ++++
 arch/x86/include/asm/disabled-features.h      |  8 ++++-
 arch/x86/kernel/cet.c                         | 36 +++++++++++++++++++
 arch/x86/kernel/cpu/common.c                  | 17 +++++++++
 .../arch/x86/include/asm/disabled-features.h  |  8 ++++-
 5 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cet.h b/arch/x86/include/asm/cet.h
index 2df357dffd24..89330e4159a9 100644
--- a/arch/x86/include/asm/cet.h
+++ b/arch/x86/include/asm/cet.h
@@ -14,8 +14,11 @@ struct sc_ext;
 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
@@ -27,6 +30,8 @@ void cet_disable_shstk(void);
 void cet_disable_free_shstk(struct task_struct *p);
 int cet_restore_signal(bool ia32, struct sc_ext *sc);
 int cet_setup_signal(bool ia32, unsigned long rstor, struct sc_ext *sc);
+int cet_setup_ibt(void);
+void cet_disable_ibt(void);
 #else
 static inline int prctl_cet(int option, unsigned long arg2) { return -EINVAL; }
 static inline int cet_setup_shstk(void) { return -EINVAL; }
@@ -37,6 +42,8 @@ static inline void cet_disable_free_shstk(struct task_struct *p) {}
 static inline int cet_restore_signal(bool ia32, struct sc_ext *sc) { return -EINVAL; }
 static inline int cet_setup_signal(bool ia32, unsigned long rstor,
 				   struct sc_ext *sc) { return -EINVAL; }
+static inline int cet_setup_ibt(void) { return -EINVAL; }
+static inline void cet_disable_ibt(void) {}
 #endif
 
 #define cpu_x86_cet_enabled() \
diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index 06323ebed643..fc7d3d5a1bf4 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -68,6 +68,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
  */
@@ -89,7 +95,7 @@
 #define DISABLED_MASK15	0
 #define DISABLED_MASK16	(DISABLE_PKU|DISABLE_OSPKE|DISABLE_LA57|DISABLE_UMIP|DISABLE_SHSTK)
 #define DISABLED_MASK17	0
-#define DISABLED_MASK18	0
+#define DISABLED_MASK18	(DISABLE_IBT)
 #define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
 
 #endif /* _ASM_X86_DISABLED_FEATURES_H */
diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
index 0004333f8373..14ad25b8ff21 100644
--- a/arch/x86/kernel/cet.c
+++ b/arch/x86/kernel/cet.c
@@ -13,6 +13,8 @@
 #include <linux/uaccess.h>
 #include <linux/sched/signal.h>
 #include <linux/compat.h>
+#include <linux/vmalloc.h>
+#include <linux/bitops.h>
 #include <asm/msr.h>
 #include <asm/user.h>
 #include <asm/fpu/internal.h>
@@ -325,3 +327,37 @@ int cet_setup_signal(bool ia32, unsigned long rstor_addr, struct sc_ext *sc_ext)
 	modify_fpu_regs_end();
 	return 0;
 }
+
+int cet_setup_ibt(void)
+{
+	u64 r;
+
+	if (!cpu_feature_enabled(X86_FEATURE_IBT))
+		return -EOPNOTSUPP;
+
+	modify_fpu_regs_begin();
+	rdmsrl(MSR_IA32_U_CET, r);
+	r |= (MSR_IA32_CET_ENDBR_EN | MSR_IA32_CET_NO_TRACK_EN);
+	wrmsrl(MSR_IA32_U_CET, r);
+	modify_fpu_regs_end();
+
+	current->thread.cet.ibt_enabled = 1;
+	return 0;
+}
+
+void cet_disable_ibt(void)
+{
+	u64 r;
+
+	if (!cpu_feature_enabled(X86_FEATURE_IBT))
+		return;
+
+	modify_fpu_regs_begin();
+	rdmsrl(MSR_IA32_U_CET, r);
+	r &= ~(MSR_IA32_CET_ENDBR_EN | MSR_IA32_CET_LEG_IW_EN |
+	       MSR_IA32_CET_NO_TRACK_EN | MSR_IA32_CET_BITMAP_MASK);
+	wrmsrl(MSR_IA32_U_CET, r);
+	modify_fpu_regs_end();
+
+	current->thread.cet.ibt_enabled = 0;
+}
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index b0780fe8717e..7fa38e4a9e82 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -441,6 +441,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 (s[0] != '\0')
+		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/tools/arch/x86/include/asm/disabled-features.h b/tools/arch/x86/include/asm/disabled-features.h
index 06323ebed643..fc7d3d5a1bf4 100644
--- a/tools/arch/x86/include/asm/disabled-features.h
+++ b/tools/arch/x86/include/asm/disabled-features.h
@@ -68,6 +68,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
  */
@@ -89,7 +95,7 @@
 #define DISABLED_MASK15	0
 #define DISABLED_MASK16	(DISABLE_PKU|DISABLE_OSPKE|DISABLE_LA57|DISABLE_UMIP|DISABLE_SHSTK)
 #define DISABLED_MASK17	0
-#define DISABLED_MASK18	0
+#define DISABLED_MASK18	(DISABLE_IBT)
 #define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
 
 #endif /* _ASM_X86_DISABLED_FEATURES_H */
-- 
2.17.1

[PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

Indirect Branch Tracking (IBT) provides an optional legacy code bitmap
that allows execution of legacy, non-IBT compatible library by an
IBT-enabled application.  When set, each bit in the bitmap indicates
one page of legacy code.

The bitmap is allocated and setup from the application.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/include/asm/cet.h |  1 +
 arch/x86/kernel/cet.c      | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/arch/x86/include/asm/cet.h b/arch/x86/include/asm/cet.h
index 89330e4159a9..9e613a6598c9 100644
--- a/arch/x86/include/asm/cet.h
+++ b/arch/x86/include/asm/cet.h
@@ -31,6 +31,7 @@ void cet_disable_free_shstk(struct task_struct *p);
 int cet_restore_signal(bool ia32, struct sc_ext *sc);
 int cet_setup_signal(bool ia32, unsigned long rstor, struct sc_ext *sc);
 int cet_setup_ibt(void);
+int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size);
 void cet_disable_ibt(void);
 #else
 static inline int prctl_cet(int option, unsigned long arg2) { return -EINVAL; }
diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
index 14ad25b8ff21..e0ef996d3148 100644
--- a/arch/x86/kernel/cet.c
+++ b/arch/x86/kernel/cet.c
@@ -22,6 +22,7 @@
 #include <asm/fpu/types.h>
 #include <asm/cet.h>
 #include <asm/special_insns.h>
+#include <asm/elf.h>
 #include <uapi/asm/sigcontext.h>
 
 static int set_shstk_ptr(unsigned long addr)
@@ -361,3 +362,28 @@ void cet_disable_ibt(void)
 
 	current->thread.cet.ibt_enabled = 0;
 }
+
+int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size)
+{
+	u64 r;
+
+	if (!current->thread.cet.ibt_enabled)
+		return -EINVAL;
+
+	if (!PAGE_ALIGNED(bitmap) || (size > TASK_SIZE_MAX))
+		return -EINVAL;
+
+	current->thread.cet.ibt_bitmap_addr = bitmap;
+	current->thread.cet.ibt_bitmap_size = size;
+
+	/*
+	 * Turn on IBT legacy bitmap.
+	 */
+	modify_fpu_regs_begin();
+	rdmsrl(MSR_IA32_U_CET, r);
+	r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
+	wrmsrl(MSR_IA32_U_CET, r);
+	modify_fpu_regs_end();
+
+	return 0;
+}
-- 
2.17.1

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Peter Zijlstra]

On Thu, Jun 06, 2019 at 01:09:15PM -0700, Yu-cheng Yu wrote:
> Indirect Branch Tracking (IBT) provides an optional legacy code bitmap
> that allows execution of legacy, non-IBT compatible library by an
> IBT-enabled application.  When set, each bit in the bitmap indicates
> one page of legacy code.
> 
> The bitmap is allocated and setup from the application.

> +int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size)
> +{
> +	u64 r;
> +
> +	if (!current->thread.cet.ibt_enabled)
> +		return -EINVAL;
> +
> +	if (!PAGE_ALIGNED(bitmap) || (size > TASK_SIZE_MAX))
> +		return -EINVAL;
> +
> +	current->thread.cet.ibt_bitmap_addr = bitmap;
> +	current->thread.cet.ibt_bitmap_size = size;
> +
> +	/*
> +	 * Turn on IBT legacy bitmap.
> +	 */
> +	modify_fpu_regs_begin();
> +	rdmsrl(MSR_IA32_U_CET, r);
> +	r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> +	wrmsrl(MSR_IA32_U_CET, r);
> +	modify_fpu_regs_end();
> +
> +	return 0;
> +}

So you just program a random user supplied address into the hardware.
What happens if there's not actually anything at that address or the
user munmap()s the data after doing this?

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Fri, 2019-06-07 at 10:08 +0200, Peter Zijlstra wrote:
> On Thu, Jun 06, 2019 at 01:09:15PM -0700, Yu-cheng Yu wrote:
> > Indirect Branch Tracking (IBT) provides an optional legacy code bitmap
> > that allows execution of legacy, non-IBT compatible library by an
> > IBT-enabled application.  When set, each bit in the bitmap indicates
> > one page of legacy code.
> > 
> > The bitmap is allocated and setup from the application.
> > +int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size)
> > +{
> > +	u64 r;
> > +
> > +	if (!current->thread.cet.ibt_enabled)
> > +		return -EINVAL;
> > +
> > +	if (!PAGE_ALIGNED(bitmap) || (size > TASK_SIZE_MAX))
> > +		return -EINVAL;
> > +
> > +	current->thread.cet.ibt_bitmap_addr = bitmap;
> > +	current->thread.cet.ibt_bitmap_size = size;
> > +
> > +	/*
> > +	 * Turn on IBT legacy bitmap.
> > +	 */
> > +	modify_fpu_regs_begin();
> > +	rdmsrl(MSR_IA32_U_CET, r);
> > +	r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> > +	wrmsrl(MSR_IA32_U_CET, r);
> > +	modify_fpu_regs_end();
> > +
> > +	return 0;
> > +}
> 
> So you just program a random user supplied address into the hardware.
> What happens if there's not actually anything at that address or the
> user munmap()s the data after doing this?

This function checks the bitmap's alignment and size, and anything else is the
app's responsibility.  What else do you think the kernel should check?

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Andy Lutomirski]

> On Jun 7, 2019, at 9:23 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> 
>> On Fri, 2019-06-07 at 10:08 +0200, Peter Zijlstra wrote:
>>> On Thu, Jun 06, 2019 at 01:09:15PM -0700, Yu-cheng Yu wrote:
>>> Indirect Branch Tracking (IBT) provides an optional legacy code bitmap
>>> that allows execution of legacy, non-IBT compatible library by an
>>> IBT-enabled application.  When set, each bit in the bitmap indicates
>>> one page of legacy code.
>>> 
>>> The bitmap is allocated and setup from the application.
>>> +int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size)
>>> +{
>>> +    u64 r;
>>> +
>>> +    if (!current->thread.cet.ibt_enabled)
>>> +        return -EINVAL;
>>> +
>>> +    if (!PAGE_ALIGNED(bitmap) || (size > TASK_SIZE_MAX))
>>> +        return -EINVAL;
>>> +
>>> +    current->thread.cet.ibt_bitmap_addr = bitmap;
>>> +    current->thread.cet.ibt_bitmap_size = size;
>>> +
>>> +    /*
>>> +     * Turn on IBT legacy bitmap.
>>> +     */
>>> +    modify_fpu_regs_begin();
>>> +    rdmsrl(MSR_IA32_U_CET, r);
>>> +    r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
>>> +    wrmsrl(MSR_IA32_U_CET, r);
>>> +    modify_fpu_regs_end();
>>> +
>>> +    return 0;
>>> +}
>> 
>> So you just program a random user supplied address into the hardware.
>> What happens if there's not actually anything at that address or the
>> user munmap()s the data after doing this?
> 
> This function checks the bitmap's alignment and size, and anything else is the
> app's responsibility.  What else do you think the kernel should check?
> 

One might reasonably wonder why this state is privileged in the first place and, given that, why we’re allowing it to be written like this.

Arguably we should have another prctl to lock these values (until exec) as a gardening measure.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/7/19 9:35 AM, Andy Lutomirski wrote:
> One might reasonably wonder why this state is privileged in the first
> place and, given that, why we’re allowing it to be written like
> this.

I think it's generally a good architectural practice to make things like
this privileged.  They're infrequent so can survive the cost of a trip
in/out of the kernel and are a great choke point to make sure the OS is
involved.  I wish we had the same for MPX or pkeys per-task "setup".

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Fri, 2019-06-07 at 09:35 -0700, Andy Lutomirski wrote:
> > On Jun 7, 2019, at 9:23 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > 
> > > On Fri, 2019-06-07 at 10:08 +0200, Peter Zijlstra wrote:
> > > > On Thu, Jun 06, 2019 at 01:09:15PM -0700, Yu-cheng Yu wrote:
> > > > Indirect Branch Tracking (IBT) provides an optional legacy code bitmap
> > > > that allows execution of legacy, non-IBT compatible library by an
> > > > IBT-enabled application.  When set, each bit in the bitmap indicates
> > > > one page of legacy code.
> > > > 
> > > > The bitmap is allocated and setup from the application.
> > > > +int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size)
> > > > +{
> > > > +    u64 r;
> > > > +
> > > > +    if (!current->thread.cet.ibt_enabled)
> > > > +        return -EINVAL;
> > > > +
> > > > +    if (!PAGE_ALIGNED(bitmap) || (size > TASK_SIZE_MAX))
> > > > +        return -EINVAL;
> > > > +
> > > > +    current->thread.cet.ibt_bitmap_addr = bitmap;
> > > > +    current->thread.cet.ibt_bitmap_size = size;
> > > > +
> > > > +    /*
> > > > +     * Turn on IBT legacy bitmap.
> > > > +     */
> > > > +    modify_fpu_regs_begin();
> > > > +    rdmsrl(MSR_IA32_U_CET, r);
> > > > +    r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> > > > +    wrmsrl(MSR_IA32_U_CET, r);
> > > > +    modify_fpu_regs_end();
> > > > +
> > > > +    return 0;
> > > > +}
> > > 
> > > So you just program a random user supplied address into the hardware.
> > > What happens if there's not actually anything at that address or the
> > > user munmap()s the data after doing this?
> > 
> > This function checks the bitmap's alignment and size, and anything else is
> > the
> > app's responsibility.  What else do you think the kernel should check?
> > 
> 
> One might reasonably wonder why this state is privileged in the first place
> and, given that, why we’re allowing it to be written like this.
> 
> Arguably we should have another prctl to lock these values (until exec) as a
> gardening measure.

We can prevent the bitmap from being set more than once.  I will test it.

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Andy Lutomirski]

> On Jun 7, 2019, at 9:45 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> 
> On Fri, 2019-06-07 at 09:35 -0700, Andy Lutomirski wrote:
>>> On Jun 7, 2019, at 9:23 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>>> 
>>>>> On Fri, 2019-06-07 at 10:08 +0200, Peter Zijlstra wrote:
>>>>> On Thu, Jun 06, 2019 at 01:09:15PM -0700, Yu-cheng Yu wrote:
>>>>> Indirect Branch Tracking (IBT) provides an optional legacy code bitmap
>>>>> that allows execution of legacy, non-IBT compatible library by an
>>>>> IBT-enabled application.  When set, each bit in the bitmap indicates
>>>>> one page of legacy code.
>>>>> 
>>>>> The bitmap is allocated and setup from the application.
>>>>> +int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size)
>>>>> +{
>>>>> +    u64 r;
>>>>> +
>>>>> +    if (!current->thread.cet.ibt_enabled)
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    if (!PAGE_ALIGNED(bitmap) || (size > TASK_SIZE_MAX))
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    current->thread.cet.ibt_bitmap_addr = bitmap;
>>>>> +    current->thread.cet.ibt_bitmap_size = size;
>>>>> +
>>>>> +    /*
>>>>> +     * Turn on IBT legacy bitmap.
>>>>> +     */
>>>>> +    modify_fpu_regs_begin();
>>>>> +    rdmsrl(MSR_IA32_U_CET, r);
>>>>> +    r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
>>>>> +    wrmsrl(MSR_IA32_U_CET, r);
>>>>> +    modify_fpu_regs_end();
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>> 
>>>> So you just program a random user supplied address into the hardware.
>>>> What happens if there's not actually anything at that address or the
>>>> user munmap()s the data after doing this?
>>> 
>>> This function checks the bitmap's alignment and size, and anything else is
>>> the
>>> app's responsibility.  What else do you think the kernel should check?
>>> 
>> 
>> One might reasonably wonder why this state is privileged in the first place
>> and, given that, why we’re allowing it to be written like this.
>> 
>> Arguably we should have another prctl to lock these values (until exec) as a
>> gardening measure.
> 
> We can prevent the bitmap from being set more than once.  I will test it.
> 

I think it would be better to make locking an explicit opt-in.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Peter Zijlstra]

On Fri, Jun 07, 2019 at 09:23:43AM -0700, Yu-cheng Yu wrote:
> On Fri, 2019-06-07 at 10:08 +0200, Peter Zijlstra wrote:
> > On Thu, Jun 06, 2019 at 01:09:15PM -0700, Yu-cheng Yu wrote:
> > > Indirect Branch Tracking (IBT) provides an optional legacy code bitmap
> > > that allows execution of legacy, non-IBT compatible library by an
> > > IBT-enabled application.  When set, each bit in the bitmap indicates
> > > one page of legacy code.
> > > 
> > > The bitmap is allocated and setup from the application.
> > > +int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size)
> > > +{
> > > +	u64 r;
> > > +
> > > +	if (!current->thread.cet.ibt_enabled)
> > > +		return -EINVAL;
> > > +
> > > +	if (!PAGE_ALIGNED(bitmap) || (size > TASK_SIZE_MAX))
> > > +		return -EINVAL;
> > > +
> > > +	current->thread.cet.ibt_bitmap_addr = bitmap;
> > > +	current->thread.cet.ibt_bitmap_size = size;
> > > +
> > > +	/*
> > > +	 * Turn on IBT legacy bitmap.
> > > +	 */
> > > +	modify_fpu_regs_begin();
> > > +	rdmsrl(MSR_IA32_U_CET, r);
> > > +	r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> > > +	wrmsrl(MSR_IA32_U_CET, r);
> > > +	modify_fpu_regs_end();
> > > +
> > > +	return 0;
> > > +}
> > 
> > So you just program a random user supplied address into the hardware.
> > What happens if there's not actually anything at that address or the
> > user munmap()s the data after doing this?
> 
> This function checks the bitmap's alignment and size, and anything else is the
> app's responsibility.  What else do you think the kernel should check?

I've no idea what the kernel should do; since you failed to answer the
question what happens when you point this to garbage.

Does it then fault or what?

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/7/19 10:43 AM, Peter Zijlstra wrote:
> I've no idea what the kernel should do; since you failed to answer the
> question what happens when you point this to garbage.
> 
> Does it then fault or what?

Yeah, I think you'll fault with a rather mysterious CR2 value since
you'll go look at the instruction that faulted and not see any
references to the CR2 value.

I think this new MSR probably needs to get included in oops output when
CET is enabled.

Why don't we require that a VMA be in place for the entire bitmap?
Don't we need a "get" prctl function too in case something like a JIT is
running and needs to find the location of this bitmap to set bits itself?

Or, do we just go whole-hog and have the kernel manage the bitmap
itself. Our interface here could be:

	prctl(PR_MARK_CODE_AS_LEGACY, start, size);

and then have the kernel allocate and set the bitmap for those code
locations.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Andy Lutomirski]

> On Jun 7, 2019, at 10:59 AM, Dave Hansen <dave.hansen@intel.com> wrote:
> 
>> On 6/7/19 10:43 AM, Peter Zijlstra wrote:
>> I've no idea what the kernel should do; since you failed to answer the
>> question what happens when you point this to garbage.
>> 
>> Does it then fault or what?
> 
> Yeah, I think you'll fault with a rather mysterious CR2 value since
> you'll go look at the instruction that faulted and not see any
> references to the CR2 value.
> 
> I think this new MSR probably needs to get included in oops output when
> CET is enabled.

This shouldn’t be able to OOPS because it only happens at CPL 3, right?  We should put it into core dumps, though.

> 
> Why don't we require that a VMA be in place for the entire bitmap?
> Don't we need a "get" prctl function too in case something like a JIT is
> running and needs to find the location of this bitmap to set bits itself?
> 
> Or, do we just go whole-hog and have the kernel manage the bitmap
> itself. Our interface here could be:
> 
>    prctl(PR_MARK_CODE_AS_LEGACY, start, size);
> 
> and then have the kernel allocate and set the bitmap for those code
> locations.

Given that the format depends on the VA size, this might be a good idea.  I bet we can reuse the special mapping infrastructure for this — the VMA could
be a MAP_PRIVATE special mapping named [cet_legacy_bitmap] or similar, and we can even make special rules to core dump it intelligently if needed.  And we can make mremap() on it work correctly if anyone (CRIU?) cares.

Hmm.  Can we be creative and skip populating it with zeros?  The CPU should only ever touch a page if we miss an ENDBR on it, so, in normal operation, we don’t need anything to be there.  We could try to prevent anyone from *reading* it outside of ENDBR tracking if we want to avoid people accidentally wasting lots of memory by forcing it to be fully populated when the read it.

The one downside is this forces it to be per-mm, but that seems like a generally reasonable model anyway.

This also gives us an excellent opportunity to make it read-only as seen from userspace to prevent exploits from just poking it full of ones before redirecting execution.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/7/19 11:29 AM, Andy Lutomirski wrote:
...
>> I think this new MSR probably needs to get included in oops output when
>> CET is enabled.
> 
> This shouldn’t be able to OOPS because it only happens at CPL 3,
> right?  We should put it into core dumps, though.

Good point.

Yu-cheng, can you just confirm that the bitmap can't be referenced in
ring-0, no matter what?  We should also make sure that no funny business
happens if we put an address in the bitmap that faults, or is
non-canonical.  Do we have any self-tests for that?

Let's say userspace gets a fault on this.  Do they have the
introspection capability to figure out why they faulted, say in their
signal handler?

>> Why don't we require that a VMA be in place for the entire bitmap?
>> Don't we need a "get" prctl function too in case something like a JIT is
>> running and needs to find the location of this bitmap to set bits itself?
>>
>> Or, do we just go whole-hog and have the kernel manage the bitmap
>> itself. Our interface here could be:
>>
>>    prctl(PR_MARK_CODE_AS_LEGACY, start, size);
>>
>> and then have the kernel allocate and set the bitmap for those code
>> locations.
> 
> Given that the format depends on the VA size, this might be a good
> idea.

Yeah, making userspace know how large the address space is or could be
is rather nasty, especially if we ever get any fancy CPU features that
eat up address bits (a la ARM top-byte-ignore or SPARC ADI).

> Hmm.  Can we be creative and skip populating it with zeros?  The CPU
should only ever touch a page if we miss an ENDBR on it, so, in normal
operation, we don’t need anything to be there.  We could try to prevent
anyone from *reading* it outside of ENDBR tracking if we want to avoid
people accidentally wasting lots of memory by forcing it to be fully
populated when the read it.

Won't reads on a big, contiguous private mapping get the huge zero page
anyway?

> The one downside is this forces it to be per-mm, but that seems like
> a generally reasonable model anyway.

Yeah, practically, you could only make it shared if you shared the
layout of all code in the address space.  I'm sure the big database(s)
do that cross-process, but I bet nobody else does.  User ASLR
practically guarantees that nobody can do this.

> This also gives us an excellent opportunity to make it read-only as
> seen from userspace to prevent exploits from just poking it full of
> ones before redirecting execution.

That would be fun.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Fri, 2019-06-07 at 11:58 -0700, Dave Hansen wrote:
> On 6/7/19 11:29 AM, Andy Lutomirski wrote:
> ...
> > > I think this new MSR probably needs to get included in oops output when
> > > CET is enabled.
> > 
> > This shouldn’t be able to OOPS because it only happens at CPL 3,
> > right?  We should put it into core dumps, though.
> 
> Good point.
> 
> Yu-cheng, can you just confirm that the bitmap can't be referenced in
> ring-0, no matter what?  We should also make sure that no funny business
> happens if we put an address in the bitmap that faults, or is
> non-canonical.  Do we have any self-tests for that?

Yes, the bitmap is user memory, but the kernel can still get to it (e.g.
copy_from_user()).  We can do more check on the address.

> 
> Let's say userspace gets a fault on this.  Do they have the
> introspection capability to figure out why they faulted, say in their
> signal handler?

The bitmap address is kept by the application; the kernel won't provide it again
to user-space.  In the signal handler, the app can find out from its own record.

[...]

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Andy Lutomirski]

> On Jun 7, 2019, at 11:58 AM, Dave Hansen <dave.hansen@intel.com> wrote:
> 
> On 6/7/19 11:29 AM, Andy Lutomirski wrote:
> ...
>>> I think this new MSR probably needs to get included in oops output when
>>> CET is enabled.
>> 
>> This shouldn’t be able to OOPS because it only happens at CPL 3,
>> right?  We should put it into core dumps, though.
> 
> Good point.
> 
> Yu-cheng, can you just confirm that the bitmap can't be referenced in
> ring-0, no matter what?  We should also make sure that no funny business
> happens if we put an address in the bitmap that faults, or is
> non-canonical.  Do we have any self-tests for that?
> 
> Let's say userspace gets a fault on this.  Do they have the
> introspection capability to figure out why they faulted, say in their
> signal handler?

We need to stick the tracker state in the sigcontext somewhere.

Did we end up defining a signal frame shadow stack token?

> 
>>> Why don't we require that a VMA be in place for the entire bitmap?
>>> Don't we need a "get" prctl function too in case something like a JIT is
>>> running and needs to find the location of this bitmap to set bits itself?
>>> 
>>> Or, do we just go whole-hog and have the kernel manage the bitmap
>>> itself. Our interface here could be:
>>> 
>>>   prctl(PR_MARK_CODE_AS_LEGACY, start, size);
>>> 
>>> and then have the kernel allocate and set the bitmap for those code
>>> locations.
>> 
>> Given that the format depends on the VA size, this might be a good
>> idea.
> 
> Yeah, making userspace know how large the address space is or could be
> is rather nasty, especially if we ever get any fancy CPU features that
> eat up address bits (a la ARM top-byte-ignore or SPARC ADI).

That gets extra bad if we ever grow user code that uses it but is unaware. It could poke the wrong part of the bitmap.

> 
>> Hmm.  Can we be creative and skip populating it with zeros?  The CPU
> should only ever touch a page if we miss an ENDBR on it, so, in normal
> operation, we don’t need anything to be there.  We could try to prevent
> anyone from *reading* it outside of ENDBR tracking if we want to avoid
> people accidentally wasting lots of memory by forcing it to be fully
> populated when the read it.
> 
> Won't reads on a big, contiguous private mapping get the huge zero page
> anyway?

The zero pages may be free, but the page tables could be decently large.  Does the core mm code use huge, immense, etc huge zero pages?  Or can it synthesize them by reusing page table pages that map zeros?

> 
>> The one downside is this forces it to be per-mm, but that seems like
>> a generally reasonable model anyway.
> 
> Yeah, practically, you could only make it shared if you shared the
> layout of all code in the address space.  I'm sure the big database(s)
> do that cross-process, but I bet nobody else does.  User ASLR
> practically guarantees that nobody can do this.

I meant per-mm instead of per-task.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/7/19 1:40 PM, Andy Lutomirski wrote:
>>> Hmm.  Can we be creative and skip populating it with zeros?  The
>>> CPU
>> should only ever touch a page if we miss an ENDBR on it, so, in
>> normal operation, we don’t need anything to be there.  We could try
>> to prevent anyone from *reading* it outside of ENDBR tracking if we
>> want to avoid people accidentally wasting lots of memory by forcing
>> it to be fully populated when the read it.
>> 
>> Won't reads on a big, contiguous private mapping get the huge zero
>> page anyway?
> 
> The zero pages may be free, but the page tables could be decently
large.  Does the core mm code use huge, immense, etc huge zero pages?
Or can it synthesize them by reusing page table pages that map zeros?

IIRC, we only ever fill single PMDs, even though we could gang a pmd
page up and do it for 1GB areas too.

I guess the page table consumption could really suck if we had code all
over the 57-bit address space and that code moved around and the process
ran for a long long time.  Pathologically, we need a ulong/pmd_t for
each 2MB of address space which is 8*2^56-30=512GB per process.  Yikes.
 Right now, we'd at least detect the memory consumption and OOM-kill the
process(es) eventually.  But, that's not really _this_ patch's problem.
 It's a general problem, and doesn't even require the zero page to be
mapped all over.

Longer-term, I'd much rather see us add some page table reclaim
mechanism that new how to go after things like excessive page tables  in
MAP_NORESERVE areas.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Fri, 2019-06-07 at 11:29 -0700, Andy Lutomirski wrote:
> > On Jun 7, 2019, at 10:59 AM, Dave Hansen <dave.hansen@intel.com> wrote:
> > 
> > > On 6/7/19 10:43 AM, Peter Zijlstra wrote:
> > > I've no idea what the kernel should do; since you failed to answer the
> > > question what happens when you point this to garbage.
> > > 
> > > Does it then fault or what?
> > 
> > Yeah, I think you'll fault with a rather mysterious CR2 value since
> > you'll go look at the instruction that faulted and not see any
> > references to the CR2 value.
> > 
> > I think this new MSR probably needs to get included in oops output when
> > CET is enabled.
> 
> This shouldn’t be able to OOPS because it only happens at CPL 3, right?  We
> should put it into core dumps, though.
> 
> > 
> > Why don't we require that a VMA be in place for the entire bitmap?
> > Don't we need a "get" prctl function too in case something like a JIT is
> > running and needs to find the location of this bitmap to set bits itself?
> > 
> > Or, do we just go whole-hog and have the kernel manage the bitmap
> > itself. Our interface here could be:
> > 
> >    prctl(PR_MARK_CODE_AS_LEGACY, start, size);
> > 
> > and then have the kernel allocate and set the bitmap for those code
> > locations.
> 
> Given that the format depends on the VA size, this might be a good idea.  I
> bet we can reuse the special mapping infrastructure for this — the VMA could
> be a MAP_PRIVATE special mapping named [cet_legacy_bitmap] or similar, and we
> can even make special rules to core dump it intelligently if needed.  And we
> can make mremap() on it work correctly if anyone (CRIU?) cares.
> 
> Hmm.  Can we be creative and skip populating it with zeros?  The CPU should
> only ever touch a page if we miss an ENDBR on it, so, in normal operation, we
> don’t need anything to be there.  We could try to prevent anyone from
> *reading* it outside of ENDBR tracking if we want to avoid people accidentally
> wasting lots of memory by forcing it to be fully populated when the read it.
> 
> The one downside is this forces it to be per-mm, but that seems like a
> generally reasonable model anyway.
> 
> This also gives us an excellent opportunity to make it read-only as seen from
> userspace to prevent exploits from just poking it full of ones before
> redirecting execution.

GLIBC sets bits only for legacy code, and then makes the bitmap read-only.  That
avoids most issues:

  To populate bitmap pages, mprotect() is required.
  Reading zero bitmap pages would not waste more physical memory, right?

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/7/19 12:49 PM, Yu-cheng Yu wrote:
>>
>> This also gives us an excellent opportunity to make it read-only as seen from
>> userspace to prevent exploits from just poking it full of ones before
>> redirecting execution.
> GLIBC sets bits only for legacy code, and then makes the bitmap read-only.  That
> avoids most issues:
> 
>   To populate bitmap pages, mprotect() is required.
>   Reading zero bitmap pages would not waste more physical memory, right?

Huh, how does glibc know about all possible past and future legacy code
in the application?

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Fri, 2019-06-07 at 13:00 -0700, Dave Hansen wrote:
> On 6/7/19 12:49 PM, Yu-cheng Yu wrote:
> > > 
> > > This also gives us an excellent opportunity to make it read-only as seen
> > > from
> > > userspace to prevent exploits from just poking it full of ones before
> > > redirecting execution.
> > 
> > GLIBC sets bits only for legacy code, and then makes the bitmap read-
> > only.  That
> > avoids most issues:
> > 
> >   To populate bitmap pages, mprotect() is required.
> >   Reading zero bitmap pages would not waste more physical memory, right?
> 
> Huh, how does glibc know about all possible past and future legacy code
> in the application?

When dlopen() gets a legacy binary and the policy allows that, it will manage
the bitmap:

  If a bitmap has not been created, create one.
  Set bits for the legacy code being loaded.

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/7/19 1:06 PM, Yu-cheng Yu wrote:
>> Huh, how does glibc know about all possible past and future legacy code
>> in the application?
> When dlopen() gets a legacy binary and the policy allows that, it will manage
> the bitmap:
> 
>   If a bitmap has not been created, create one.
>   Set bits for the legacy code being loaded.

I was thinking about code that doesn't go through GLIBC like JITs.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Andy Lutomirski]

> On Jun 7, 2019, at 2:09 PM, Dave Hansen <dave.hansen@intel.com> wrote:
> 
> On 6/7/19 1:06 PM, Yu-cheng Yu wrote:
>>> Huh, how does glibc know about all possible past and future legacy code
>>> in the application?
>> When dlopen() gets a legacy binary and the policy allows that, it will manage
>> the bitmap:
>> 
>>  If a bitmap has not been created, create one.
>>  Set bits for the legacy code being loaded.
> 
> I was thinking about code that doesn't go through GLIBC like JITs.

CRIU is another consideration: it would be rather annoying if CET programs can’t migrate between LA57 and normal machines.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Fri, 2019-06-07 at 15:27 -0700, Andy Lutomirski wrote:
> > On Jun 7, 2019, at 2:09 PM, Dave Hansen <dave.hansen@intel.com> wrote:
> > 
> > On 6/7/19 1:06 PM, Yu-cheng Yu wrote:
> > > > Huh, how does glibc know about all possible past and future legacy code
> > > > in the application?
> > > 
> > > When dlopen() gets a legacy binary and the policy allows that, it will
> > > manage
> > > the bitmap:
> > > 
> > >  If a bitmap has not been created, create one.
> > >  Set bits for the legacy code being loaded.
> > 
> > I was thinking about code that doesn't go through GLIBC like JITs.
> 
> CRIU is another consideration: it would be rather annoying if CET programs
> can’t migrate between LA57 and normal machines.

When a machine migrates, does its applications' addresses change?
If no, then the bitmap should still work, right?

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Fri, 2019-06-07 at 14:09 -0700, Dave Hansen wrote:
> On 6/7/19 1:06 PM, Yu-cheng Yu wrote:
> > > Huh, how does glibc know about all possible past and future legacy code
> > > in the application?
> > 
> > When dlopen() gets a legacy binary and the policy allows that, it will
> > manage
> > the bitmap:
> > 
> >   If a bitmap has not been created, create one.
> >   Set bits for the legacy code being loaded.
> 
> I was thinking about code that doesn't go through GLIBC like JITs.

If JIT manages the bitmap, it knows where it is.
It can always read the bitmap again, right?

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Florian Weimer]

* Yu-cheng Yu:

> On Fri, 2019-06-07 at 14:09 -0700, Dave Hansen wrote:
>> On 6/7/19 1:06 PM, Yu-cheng Yu wrote:
>> > > Huh, how does glibc know about all possible past and future legacy code
>> > > in the application?
>> > 
>> > When dlopen() gets a legacy binary and the policy allows that, it will
>> > manage
>> > the bitmap:
>> > 
>> >   If a bitmap has not been created, create one.
>> >   Set bits for the legacy code being loaded.
>> 
>> I was thinking about code that doesn't go through GLIBC like JITs.
>
> If JIT manages the bitmap, it knows where it is.
> It can always read the bitmap again, right?

The problem are JIT libraries without assembler code which can be marked
non-CET, such as liborc.  Our builds (e.g., orc-0.4.29-2.fc30.x86_64)
currently carries the IBT and SHSTK flag, although the entry points into
the generated code do not start with ENDBR, so that a jump to them will
fault with the CET enabled.

Thanks,
Florian

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/10/19 9:05 AM, Yu-cheng Yu wrote:
> On Fri, 2019-06-07 at 14:09 -0700, Dave Hansen wrote:
>> On 6/7/19 1:06 PM, Yu-cheng Yu wrote:
>>>> Huh, how does glibc know about all possible past and future legacy code
>>>> in the application?
>>> When dlopen() gets a legacy binary and the policy allows that, it will
>>> manage
>>> the bitmap:
>>>
>>>   If a bitmap has not been created, create one.
>>>   Set bits for the legacy code being loaded.
>> I was thinking about code that doesn't go through GLIBC like JITs.
> If JIT manages the bitmap, it knows where it is.
> It can always read the bitmap again, right?

Let's just be clear:

The design proposed here is that all code mappers (anybody wanting to
get legacy non-CET code into the address space):

1. Know about CET
2. Know where the bitmap is, and identify the part that needs to be
   changed
3. Be able to mprotect() the bitmap to be writable (undoing glibc's
   PROT_READ)
4. Set the bits in the bitmap for the legacy code
5. mprotect() the bitmap back to PROT_READ

Do the non-glibc code mappers have glibc interfaces for this?
Otherwise, how could a bunch of JITs in a big multi-threaded application
possibly coordinate the mprotect()s?  Won't they race with each other?

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Andy Lutomirski]

> On Jun 7, 2019, at 12:49 PM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> 
> On Fri, 2019-06-07 at 11:29 -0700, Andy Lutomirski wrote:
>>> On Jun 7, 2019, at 10:59 AM, Dave Hansen <dave.hansen@intel.com> wrote:
>>> 
>>>> On 6/7/19 10:43 AM, Peter Zijlstra wrote:
>>>> I've no idea what the kernel should do; since you failed to answer the
>>>> question what happens when you point this to garbage.
>>>> 
>>>> Does it then fault or what?
>>> 
>>> Yeah, I think you'll fault with a rather mysterious CR2 value since
>>> you'll go look at the instruction that faulted and not see any
>>> references to the CR2 value.
>>> 
>>> I think this new MSR probably needs to get included in oops output when
>>> CET is enabled.
>> 
>> This shouldn’t be able to OOPS because it only happens at CPL 3, right?  We
>> should put it into core dumps, though.
>> 
>>> 
>>> Why don't we require that a VMA be in place for the entire bitmap?
>>> Don't we need a "get" prctl function too in case something like a JIT is
>>> running and needs to find the location of this bitmap to set bits itself?
>>> 
>>> Or, do we just go whole-hog and have the kernel manage the bitmap
>>> itself. Our interface here could be:
>>> 
>>>   prctl(PR_MARK_CODE_AS_LEGACY, start, size);
>>> 
>>> and then have the kernel allocate and set the bitmap for those code
>>> locations.
>> 
>> Given that the format depends on the VA size, this might be a good idea.  I
>> bet we can reuse the special mapping infrastructure for this — the VMA could
>> be a MAP_PRIVATE special mapping named [cet_legacy_bitmap] or similar, and we
>> can even make special rules to core dump it intelligently if needed.  And we
>> can make mremap() on it work correctly if anyone (CRIU?) cares.
>> 
>> Hmm.  Can we be creative and skip populating it with zeros?  The CPU should
>> only ever touch a page if we miss an ENDBR on it, so, in normal operation, we
>> don’t need anything to be there.  We could try to prevent anyone from
>> *reading* it outside of ENDBR tracking if we want to avoid people accidentally
>> wasting lots of memory by forcing it to be fully populated when the read it.
>> 
>> The one downside is this forces it to be per-mm, but that seems like a
>> generally reasonable model anyway.
>> 
>> This also gives us an excellent opportunity to make it read-only as seen from
>> userspace to prevent exploits from just poking it full of ones before
>> redirecting execution.
> 
> GLIBC sets bits only for legacy code, and then makes the bitmap read-only.  That
> avoids most issues:

How does glibc know the linear address space size?  We don’t want LA64 to break old binaries because the address calculation changed.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Fri, 2019-06-07 at 13:43 -0700, Andy Lutomirski wrote:
> > On Jun 7, 2019, at 12:49 PM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > 
> > On Fri, 2019-06-07 at 11:29 -0700, Andy Lutomirski wrote:
> > > > On Jun 7, 2019, at 10:59 AM, Dave Hansen <dave.hansen@intel.com> wrote:
> > > > 
> > > > > On 6/7/19 10:43 AM, Peter Zijlstra wrote:
> > > > > I've no idea what the kernel should do; since you failed to answer the
> > > > > question what happens when you point this to garbage.
> > > > > 
> > > > > Does it then fault or what?
> > > > 
> > > > Yeah, I think you'll fault with a rather mysterious CR2 value since
> > > > you'll go look at the instruction that faulted and not see any
> > > > references to the CR2 value.
> > > > 
> > > > I think this new MSR probably needs to get included in oops output when
> > > > CET is enabled.
> > > 
> > > This shouldn’t be able to OOPS because it only happens at CPL 3,
> > > right?  We
> > > should put it into core dumps, though.
> > > 
> > > > 
> > > > Why don't we require that a VMA be in place for the entire bitmap?
> > > > Don't we need a "get" prctl function too in case something like a JIT is
> > > > running and needs to find the location of this bitmap to set bits
> > > > itself?
> > > > 
> > > > Or, do we just go whole-hog and have the kernel manage the bitmap
> > > > itself. Our interface here could be:
> > > > 
> > > >   prctl(PR_MARK_CODE_AS_LEGACY, start, size);
> > > > 
> > > > and then have the kernel allocate and set the bitmap for those code
> > > > locations.
> > > 
> > > Given that the format depends on the VA size, this might be a good
> > > idea.  I
> > > bet we can reuse the special mapping infrastructure for this — the VMA
> > > could
> > > be a MAP_PRIVATE special mapping named [cet_legacy_bitmap] or similar, and
> > > we
> > > can even make special rules to core dump it intelligently if needed.  And
> > > we
> > > can make mremap() on it work correctly if anyone (CRIU?) cares.
> > > 
> > > Hmm.  Can we be creative and skip populating it with zeros?  The CPU
> > > should
> > > only ever touch a page if we miss an ENDBR on it, so, in normal operation,
> > > we
> > > don’t need anything to be there.  We could try to prevent anyone from
> > > *reading* it outside of ENDBR tracking if we want to avoid people
> > > accidentally
> > > wasting lots of memory by forcing it to be fully populated when the read
> > > it.
> > > 
> > > The one downside is this forces it to be per-mm, but that seems like a
> > > generally reasonable model anyway.
> > > 
> > > This also gives us an excellent opportunity to make it read-only as seen
> > > from
> > > userspace to prevent exploits from just poking it full of ones before
> > > redirecting execution.
> > 
> > GLIBC sets bits only for legacy code, and then makes the bitmap read-
> > only.  That
> > avoids most issues:
> 
> How does glibc know the linear address space size?  We don’t want LA64 to
> break old binaries because the address calculation changed.

When an application starts, its highest stack address is determined.
It uses that as the maximum the bitmap needs to cover.

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/10/19 8:22 AM, Yu-cheng Yu wrote:
>> How does glibc know the linear address space size?  We don’t want LA64 to
>> break old binaries because the address calculation changed.
> When an application starts, its highest stack address is determined.
> It uses that as the maximum the bitmap needs to cover.

Huh, I didn't think we ran code from the stack. ;)

Especially given the way that we implemented the new 5-level-paging
address space, I don't think that expecting code to be below the stack
is a good universal expectation.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Mon, 2019-06-10 at 11:02 -0700, Dave Hansen wrote:
> On 6/10/19 8:22 AM, Yu-cheng Yu wrote:
> > > How does glibc know the linear address space size?  We don’t want LA64 to
> > > break old binaries because the address calculation changed.
> > 
> > When an application starts, its highest stack address is determined.
> > It uses that as the maximum the bitmap needs to cover.
> 
> Huh, I didn't think we ran code from the stack. ;)
> 
> Especially given the way that we implemented the new 5-level-paging
> address space, I don't think that expecting code to be below the stack
> is a good universal expectation.

Yes, you make a good point.  However, allowing the application manage the bitmap
is the most efficient and flexible.  If the loader finds a legacy lib is beyond
the bitmap can cover, it can deal with the problem by moving the lib to a lower
address; or re-allocate the bitmap.  If the loader cannot allocate a big bitmap
to cover all 5-level address space (the bitmap will be large), it can put all
legacy lib's at lower address.  We cannot do these easily in the kernel.

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/10/19 12:38 PM, Yu-cheng Yu wrote:
>>> When an application starts, its highest stack address is determined.
>>> It uses that as the maximum the bitmap needs to cover.
>> Huh, I didn't think we ran code from the stack. ;)
>>
>> Especially given the way that we implemented the new 5-level-paging
>> address space, I don't think that expecting code to be below the stack
>> is a good universal expectation.
> Yes, you make a good point.  However, allowing the application manage the bitmap
> is the most efficient and flexible.  If the loader finds a legacy lib is beyond
> the bitmap can cover, it can deal with the problem by moving the lib to a lower
> address; or re-allocate the bitmap.

How could the loader reallocate the bitmap and coordinate with other
users of the bitmap?

> If the loader cannot allocate a big bitmap to cover all 5-level
> address space (the bitmap will be large), it can put all legacy lib's
> at lower address.  We cannot do these easily in the kernel.

This is actually an argument to do it in the kernel.  The kernel can
always allocate the virtual space however it wants, no matter how large.
 If we hide the bitmap behind a kernel API then we can put it at high
5-level user addresses because we also don't have to worry about the
high bits confusing userspace.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Andy Lutomirski]

On Mon, Jun 10, 2019 at 12:52 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 6/10/19 12:38 PM, Yu-cheng Yu wrote:
> >>> When an application starts, its highest stack address is determined.
> >>> It uses that as the maximum the bitmap needs to cover.
> >> Huh, I didn't think we ran code from the stack. ;)
> >>
> >> Especially given the way that we implemented the new 5-level-paging
> >> address space, I don't think that expecting code to be below the stack
> >> is a good universal expectation.
> > Yes, you make a good point.  However, allowing the application manage the bitmap
> > is the most efficient and flexible.  If the loader finds a legacy lib is beyond
> > the bitmap can cover, it can deal with the problem by moving the lib to a lower
> > address; or re-allocate the bitmap.
>
> How could the loader reallocate the bitmap and coordinate with other
> users of the bitmap?
>
> > If the loader cannot allocate a big bitmap to cover all 5-level
> > address space (the bitmap will be large), it can put all legacy lib's
> > at lower address.  We cannot do these easily in the kernel.
>
> This is actually an argument to do it in the kernel.  The kernel can
> always allocate the virtual space however it wants, no matter how large.
>  If we hide the bitmap behind a kernel API then we can put it at high
> 5-level user addresses because we also don't have to worry about the
> high bits confusing userspace.
>

That's a fairly compelling argument.

The bitmap is one bit per page, right?  So it's smaller than the
address space by a factor of 8*2^12 == 2^15.  This means that, if we
ever get full 64-bit linear addresses reserved entirely for userspace
(which could happen if my perennial request to Intel to split user and
kernel addresses completely happens), then we'll need 2^48 bytes for
the bitmap, which simply does not fit in the address space of a legacy
application.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Mon, 2019-06-10 at 12:52 -0700, Dave Hansen wrote:
> On 6/10/19 12:38 PM, Yu-cheng Yu wrote:
> > > > When an application starts, its highest stack address is determined.
> > > > It uses that as the maximum the bitmap needs to cover.
> > > 
> > > Huh, I didn't think we ran code from the stack. ;)
> > > 
> > > Especially given the way that we implemented the new 5-level-paging
> > > address space, I don't think that expecting code to be below the stack
> > > is a good universal expectation.
> > 
> > Yes, you make a good point.  However, allowing the application manage the
> > bitmap
> > is the most efficient and flexible.  If the loader finds a legacy lib is
> > beyond
> > the bitmap can cover, it can deal with the problem by moving the lib to a
> > lower
> > address; or re-allocate the bitmap.
> 
> How could the loader reallocate the bitmap and coordinate with other
> users of the bitmap?

Assuming the loader actually chooses to re-allocate, it can copy the old bitmap
over to the new before doing the switch.  But, I agree, the other choice is
easier; the loader can simply put the lib at lower address.  AFAIK, the loader
does not request high address in mmap().

> 
> > If the loader cannot allocate a big bitmap to cover all 5-level
> > address space (the bitmap will be large), it can put all legacy lib's
> > at lower address.  We cannot do these easily in the kernel.
> 
> This is actually an argument to do it in the kernel.  The kernel can
> always allocate the virtual space however it wants, no matter how large.
>  If we hide the bitmap behind a kernel API then we can put it at high
> 5-level user addresses because we also don't have to worry about the
> high bits confusing userspace.

We actually tried this.  The kernel needs to reserve the bitmap space in the
beginning for every CET-enabled app, regardless of actual needs.  On each memory
request, the kernel then must consider a percentage of allocated space in its
calculation, and on systems with less memory this quickly becomes a problem.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/10/19 1:27 PM, Yu-cheng Yu wrote:
>>> If the loader cannot allocate a big bitmap to cover all 5-level
>>> address space (the bitmap will be large), it can put all legacy lib's
>>> at lower address.  We cannot do these easily in the kernel.
>> This is actually an argument to do it in the kernel.  The kernel can
>> always allocate the virtual space however it wants, no matter how large.
>>  If we hide the bitmap behind a kernel API then we can put it at high
>> 5-level user addresses because we also don't have to worry about the
>> high bits confusing userspace.
> We actually tried this.  The kernel needs to reserve the bitmap space in the
> beginning for every CET-enabled app, regardless of actual needs. 

I don't think this is a problem.  In fact, I think reserving the space
is actually the only sane behavior.  If you don't reserve it, you
fundamentally limit where future legacy instructions can go.

One idea is that we always size the bitmap for the 48-bit addressing
systems.  Legacy code probably doesn't _need_ to go in the new address
space, and if we do this we don't have to worry about the gigantic
57-bit address space bitmap.

> On each memory request, the kernel then must consider a percentage of
> allocated space in its calculation, and on systems with less memory
> this quickly becomes a problem.

I'm not sure what you're referring to here?  Are you referring to our
overcommit limits?

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Mon, 2019-06-10 at 13:43 -0700, Dave Hansen wrote:
> On 6/10/19 1:27 PM, Yu-cheng Yu wrote:
> > > > If the loader cannot allocate a big bitmap to cover all 5-level
> > > > address space (the bitmap will be large), it can put all legacy lib's
> > > > at lower address.  We cannot do these easily in the kernel.
> > > 
> > > This is actually an argument to do it in the kernel.  The kernel can
> > > always allocate the virtual space however it wants, no matter how large.
> > >  If we hide the bitmap behind a kernel API then we can put it at high
> > > 5-level user addresses because we also don't have to worry about the
> > > high bits confusing userspace.
> > 
> > We actually tried this.  The kernel needs to reserve the bitmap space in the
> > beginning for every CET-enabled app, regardless of actual needs. 
> 
> I don't think this is a problem.  In fact, I think reserving the space
> is actually the only sane behavior.  If you don't reserve it, you
> fundamentally limit where future legacy instructions can go.
> 
> One idea is that we always size the bitmap for the 48-bit addressing
> systems.  Legacy code probably doesn't _need_ to go in the new address
> space, and if we do this we don't have to worry about the gigantic
> 57-bit address space bitmap.
> 
> > On each memory request, the kernel then must consider a percentage of
> > allocated space in its calculation, and on systems with less memory
> > this quickly becomes a problem.
> 
> I'm not sure what you're referring to here?  Are you referring to our
> overcommit limits?

Yes.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/10/19 1:58 PM, Yu-cheng Yu wrote:
>>> On each memory request, the kernel then must consider a percentage of
>>> allocated space in its calculation, and on systems with less memory
>>> this quickly becomes a problem.
>> I'm not sure what you're referring to here?  Are you referring to our
>> overcommit limits?
> Yes.

My assumption has always been that these large, potentially sparse
hardware tables *must* be mmap()'d with MAP_NORESERVE specified.  That
should keep them from being problematic with respect to overcommit.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Mon, 2019-06-10 at 15:02 -0700, Dave Hansen wrote:
> On 6/10/19 1:58 PM, Yu-cheng Yu wrote:
> > > > On each memory request, the kernel then must consider a percentage of
> > > > allocated space in its calculation, and on systems with less memory
> > > > this quickly becomes a problem.
> > > 
> > > I'm not sure what you're referring to here?  Are you referring to our
> > > overcommit limits?
> > 
> > Yes.
> 
> My assumption has always been that these large, potentially sparse
> hardware tables *must* be mmap()'d with MAP_NORESERVE specified.  That
> should keep them from being problematic with respect to overcommit.

Ok, we will go back to do_mmap() with MAP_PRIVATE, MAP_NORESERVE and
VM_DONTDUMP.  The bitmap will cover only 48-bit address space.

We then create PR_MARK_CODE_AS_LEGACY.  The kernel will set the bitmap, but it
is going to be slow.

Perhaps we still let the app fill the bitmap?

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/10/19 3:40 PM, Yu-cheng Yu wrote:
> Ok, we will go back to do_mmap() with MAP_PRIVATE, MAP_NORESERVE and
> VM_DONTDUMP.  The bitmap will cover only 48-bit address space.

Could you make sure to discuss the downsides of only doing a 48-bit
address space?

What are the reasons behind and implications of VM_DONTDUMP?

> We then create PR_MARK_CODE_AS_LEGACY.  The kernel will set the bitmap, but it
> is going to be slow.

Slow compared to what?  We're effectively adding one (quick) system call
to a path that, today, has at *least* half a dozen syscalls and probably
a bunch of page faults.  Heck, we can probably avoid the actual page
fault to populate the bitmap if we're careful.  That alone would put a
syscall on equal footing with any other approach.  If the bit setting
crossed a page boundary it would probably win.

> Perhaps we still let the app fill the bitmap?

I think I'd want to see some performance data on it first.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[H.J. Lu]

On Mon, Jun 10, 2019 at 3:59 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> > We then create PR_MARK_CODE_AS_LEGACY.  The kernel will set the bitmap, but it
> > is going to be slow.
>
> Slow compared to what?  We're effectively adding one (quick) system call
> to a path that, today, has at *least* half a dozen syscalls and probably
> a bunch of page faults.  Heck, we can probably avoid the actual page
> fault to populate the bitmap if we're careful.  That alone would put a
> syscall on equal footing with any other approach.  If the bit setting
> crossed a page boundary it would probably win.
>
> > Perhaps we still let the app fill the bitmap?
>
> I think I'd want to see some performance data on it first.

Updating legacy bitmap in user space from kernel requires

long q;

get_user(q, ...);
q |= mask;
put_user(q, ...);

instead of

*p |= mask;

get_user + put_user was quite slow when we tried before.

-- 
H.J.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/10/19 4:20 PM, H.J. Lu wrote:
>>> Perhaps we still let the app fill the bitmap?
>> I think I'd want to see some performance data on it first.
> Updating legacy bitmap in user space from kernel requires
> 
> long q;
> 
> get_user(q, ...);
> q |= mask;
> put_user(q, ...);
> 
> instead of
> 
> *p |= mask;
> 
> get_user + put_user was quite slow when we tried before.

Numbers, please.

There are *lots* of ways to speed something like that up if you have
actual issues with it.  For instance, you can skip the get_user() for
whole bytes.  You can write bits with 0's for unallocated address space.
 You can do user_access_begin/end() to avoid bunches of STAC/CLACs...

The list goes on and on. :)

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Andy Lutomirski]

> On Jun 10, 2019, at 3:59 PM, Dave Hansen <dave.hansen@intel.com> wrote:
> 
>> On 6/10/19 3:40 PM, Yu-cheng Yu wrote:
>> Ok, we will go back to do_mmap() with MAP_PRIVATE, MAP_NORESERVE and
>> VM_DONTDUMP.  The bitmap will cover only 48-bit address space.
> 
> Could you make sure to discuss the downsides of only doing a 48-bit
> address space?
> 
> What are the reasons behind and implications of VM_DONTDUMP?
> 
>> We then create PR_MARK_CODE_AS_LEGACY.  The kernel will set the bitmap, but it
>> is going to be slow.
> 
> Slow compared to what?  We're effectively adding one (quick) system call
> to a path that, today, has at *least* half a dozen syscalls and probably
> a bunch of page faults.  Heck, we can probably avoid the actual page
> fault to populate the bitmap if we're careful.  That alone would put a
> syscall on equal footing with any other approach.  If the bit setting
> crossed a page boundary it would probably win.
> 
>> Perhaps we still let the app fill the bitmap?
> 
> I think I'd want to see some performance data on it first.

Trying to summarize:

If we manage the whole thing in user space, we are basically committing to only covering 48 bits — otherwise the whole model falls apart in quite a few ways. We gain some simplicity in the kernel.

If we do it in the kernel, we still have to decide how much address space to cover. We get to play games like allocating the bitmap above 2^48, but then we might have CRIU issues if we migrate to a system with fewer BA bits.

I doubt that the performance matters much one way or another. I just don’t expect any of this to be a bottleneck.

Another benefit of kernel management: we could plausibly auto-clear the bits corresponding to munmapped regions. Is this worth it?

And a maybe-silly benefit: if we manage it in the kernel, we could optimize the inevitable case where the bitmap contains pages that are all ones :). If it’s in userspace, KSM could do the, but that will be inefficient at best.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/10/19 4:54 PM, Andy Lutomirski wrote:
> Another benefit of kernel management: we could plausibly auto-clear
> the bits corresponding to munmapped regions. Is this worth it?

I did it for MPX.  I think I even went to the trouble of zapping the
whole pages that got unused.

But, MPX tables took 80% of the address space, worst-case.  This takes
0.003% :)  The only case it would really matter would be a task was
long-running, used legacy executables/JITs, and was mapping/unmapping
text all over the address space.  That seems rather unlikely.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Andy Lutomirski]

> On Jun 10, 2019, at 5:08 PM, Dave Hansen <dave.hansen@intel.com> wrote:
> 
>> On 6/10/19 4:54 PM, Andy Lutomirski wrote:
>> Another benefit of kernel management: we could plausibly auto-clear
>> the bits corresponding to munmapped regions. Is this worth it?
> 
> I did it for MPX.  I think I even went to the trouble of zapping the
> whole pages that got unused.
> 
> But, MPX tables took 80% of the address space, worst-case.  This takes
> 0.003% :)  The only case it would really matter would be a task was
> long-running, used legacy executables/JITs, and was mapping/unmapping
> text all over the address space.  That seems rather unlikely.

Every wasted page still costs 4K plus page table overhead.  The worst case is a JIT that doesn’t clean up and leaks legacy bitmap memory all over. We can blame the JIT, but the actual attribution could be complicated.

It also matters when you unmap one thing, map something else, and are sad when the legacy bits are still set.

Admittedly, it’s a bit hard to imagine the exploit that takes advantage of this.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Mon, 2019-06-10 at 15:59 -0700, Dave Hansen wrote:
> On 6/10/19 3:40 PM, Yu-cheng Yu wrote:
> > Ok, we will go back to do_mmap() with MAP_PRIVATE, MAP_NORESERVE and
> > VM_DONTDUMP.  The bitmap will cover only 48-bit address space.
> 
> Could you make sure to discuss the downsides of only doing a 48-bit
> address space?

The downside is that we cannot load legacy lib's above 48-bit address space, but
currently ld-linux does not do that.  Should ld-linux do that in the future,
dlopen() fails.  Considering CRIU migration, we probably need to do this anyway?

> What are the reasons behind and implications of VM_DONTDUMP?

The bitmap is very big.

In GDB, it should be easy to tell why a control-protection fault occurred
without the bitmap.

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/14/19 8:25 AM, Yu-cheng Yu wrote:
> On Mon, 2019-06-10 at 15:59 -0700, Dave Hansen wrote:
>> On 6/10/19 3:40 PM, Yu-cheng Yu wrote:
>>> Ok, we will go back to do_mmap() with MAP_PRIVATE, MAP_NORESERVE and
>>> VM_DONTDUMP.  The bitmap will cover only 48-bit address space.
>>
>> Could you make sure to discuss the downsides of only doing a 48-bit
>> address space?
> 
> The downside is that we cannot load legacy lib's above 48-bit address space, but
> currently ld-linux does not do that.  Should ld-linux do that in the future,
> dlopen() fails.  Considering CRIU migration, we probably need to do this anyway?

Again, I was thinking about JITs.  Please remember that not all code in
the system is from files on the disk.  Please.  We need to be really,
really sure that we don't addle this implementation by being narrow
minded about this.

Please don't forget about JITs.

>> What are the reasons behind and implications of VM_DONTDUMP?
> 
> The bitmap is very big.

Really?  It's actually, what, 8*4096=32k, so 1/32,768th of the size of
the libraries legacy libraries you load?  Do our crash dumps really not
know how to represent or deal with sparse mappings?

> In GDB, it should be easy to tell why a control-protection fault occurred
> without the bitmap.

How about why one didn't happen?

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Fri, 2019-06-14 at 09:13 -0700, Dave Hansen wrote:
> On 6/14/19 8:25 AM, Yu-cheng Yu wrote:
> > On Mon, 2019-06-10 at 15:59 -0700, Dave Hansen wrote:
> > > On 6/10/19 3:40 PM, Yu-cheng Yu wrote:
> > > > Ok, we will go back to do_mmap() with MAP_PRIVATE, MAP_NORESERVE and
> > > > VM_DONTDUMP.  The bitmap will cover only 48-bit address space.
> > > 
> > > Could you make sure to discuss the downsides of only doing a 48-bit
> > > address space?
> > 
> > The downside is that we cannot load legacy lib's above 48-bit address space,
> > but
> > currently ld-linux does not do that.  Should ld-linux do that in the future,
> > dlopen() fails.  Considering CRIU migration, we probably need to do this
> > anyway?
> 
> Again, I was thinking about JITs.  Please remember that not all code in
> the system is from files on the disk.  Please.  We need to be really,
> really sure that we don't addle this implementation by being narrow
> minded about this.
> 
> Please don't forget about JITs.
> 
> > > What are the reasons behind and implications of VM_DONTDUMP?
> > 
> > The bitmap is very big.
> 
> Really?  It's actually, what, 8*4096=32k, so 1/32,768th of the size of
> the libraries legacy libraries you load?  Do our crash dumps really not
> know how to represent or deal with sparse mappings?

Ok, even the core dump is not physically big, its size still looks odd, right?
Could this also affect how much time for GDB to load it.
We will only mmap the bitmap when the first time the bitmap prctl is called.

I have a related question:

Do we allow the application to read the bitmap, or any fault from the
application on bitmap pages?

We populate a page only when bits are set from a prctl.
Any other fault means either the application tries to find out an address
range's status or it executes legacy code that has not been marked in the
bitmap.

> 
> > In GDB, it should be easy to tell why a control-protection fault occurred
> > without the bitmap.
> 
> How about why one didn't happen?

We'll dump the bitmap if it is allocated.

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/14/19 10:13 AM, Yu-cheng Yu wrote:
> On Fri, 2019-06-14 at 09:13 -0700, Dave Hansen wrote:
>> On 6/14/19 8:25 AM, Yu-cheng Yu wrote:
>>> The bitmap is very big.
>>
>> Really?  It's actually, what, 8*4096=32k, so 1/32,768th of the size of
>> the libraries legacy libraries you load?  Do our crash dumps really not
>> know how to represent or deal with sparse mappings?
> 
> Ok, even the core dump is not physically big, its size still looks odd, right?

Hell if I know.

Could you please go try this in practice so that we're designing this
thing fixing real actual problems instead of phantoms that we're
anticipating?

> Could this also affect how much time for GDB to load it.

I don't know.  Can you go find out for sure, please?

> I have a related question:
> 
> Do we allow the application to read the bitmap, or any fault from the
> application on bitmap pages?

We have to allow apps to read it.  Otherwise they can't execute
instructions.

We don't have to allow them to (popuating) fault on it.  But, if we
don't, we need some kind of kernel interface to avoid the faults.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Fri, 2019-06-14 at 13:57 -0700, Dave Hansen wrote:
> On 6/14/19 10:13 AM, Yu-cheng Yu wrote:
> > On Fri, 2019-06-14 at 09:13 -0700, Dave Hansen wrote:
> > > On 6/14/19 8:25 AM, Yu-cheng Yu wrote:
> > > > The bitmap is very big.
> > > 
> > > Really?  It's actually, what, 8*4096=32k, so 1/32,768th of the size of
> > > the libraries legacy libraries you load?  Do our crash dumps really not
> > > know how to represent or deal with sparse mappings?
> > 
> > Ok, even the core dump is not physically big, its size still looks odd,
> > right?
> 
> Hell if I know.
> 
> Could you please go try this in practice so that we're designing this
> thing fixing real actual problems instead of phantoms that we're
> anticipating?
> 
> > Could this also affect how much time for GDB to load it.
> 
> I don't know.  Can you go find out for sure, please?

OK!

> 
> > I have a related question:
> > 
> > Do we allow the application to read the bitmap, or any fault from the
> > application on bitmap pages?
> 
> We have to allow apps to read it.  Otherwise they can't execute
> instructions.

What I meant was, if an app executes some legacy code that results in bitmap
lookup, but the bitmap page is not yet populated, and if we then populate that
page with all-zero, a #CP should follow.  So do we even populate that zero page
at all?

I think we should; a #CP is more obvious to the user at least.

> 
> We don't have to allow them to (popuating) fault on it.  But, if we
> don't, we need some kind of kernel interface to avoid the faults.

The plan is:

* Move STACK_TOP (and vdso) down to give space to the bitmap.

* Reserve the bitmap space from (mm->start_stack + PAGE_SIZE) to cover a code
size of TASK_SIZE_LOW, which is (TASK_SIZE_LOW / PAGE_SIZE / 8).

* Mmap the space only when the app issues the first mark-legacy prctl.  This
avoids the core-dump issue for most apps and the accounting problem that
MAP_NORESERVE probably won't solve completely.

* The bitmap is read-only.  The kernel sets the bitmap with
get_user_pages_fast(FOLL_WRITE) and user_access_begin()/user_addess_end().

I will send out a RFC patch.

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/14/19 2:34 PM, Yu-cheng Yu wrote:
> On Fri, 2019-06-14 at 13:57 -0700, Dave Hansen wrote:
>>> I have a related question:
>>>
>>> Do we allow the application to read the bitmap, or any fault from the
>>> application on bitmap pages?
>>
>> We have to allow apps to read it.  Otherwise they can't execute
>> instructions.
> 
> What I meant was, if an app executes some legacy code that results in bitmap
> lookup, but the bitmap page is not yet populated, and if we then populate that
> page with all-zero, a #CP should follow.  So do we even populate that zero page
> at all?
> 
> I think we should; a #CP is more obvious to the user at least.

Please make an effort to un-Intel-ificate your messages as much as
possible.  I'd really prefer that folks say "missing end branch fault"
rather than #CP.  I had to Google "#CP".

I *think* you are saying that:  The *only* lookups to this bitmap are on
"missing end branch" conditions.  Normal, proper-functioning code
execution that has ENDBR instructions in it will never even look at the
bitmap.  The only case when we reference the bitmap locations is when
the processor is about do do a "missing end branch fault" so that it can
be suppressed.  Any population with the zero page would be done when
code had already encountered a "missing end branch" condition, and
populating with a zero-filled page will guarantee that a "missing end
branch fault" will result.  You're arguing that we should just figure
this out at fault time and not ever reach the "missing end branch fault"
at all.

Is that right?

If so, that's an architecture subtlety that I missed until now and which
went entirely unmentioned in the changelog and discussion up to this
point.  Let's make sure that nobody else has to walk that path by
improving our changelog, please.

In any case, I don't think this is worth special-casing our zero-fill
code, FWIW.  It's not performance critical and not worth the complexity.
 If apps want to handle the signals and abuse this to fill space up with
boring page table contents, they're welcome to.  There are much easier
ways to consume a lot of memory.

>> We don't have to allow them to (popuating) fault on it.  But, if we
>> don't, we need some kind of kernel interface to avoid the faults.
> 
> The plan is:
> 
> * Move STACK_TOP (and vdso) down to give space to the bitmap.

Even for apps with 57-bit address spaces?

> * Reserve the bitmap space from (mm->start_stack + PAGE_SIZE) to cover a code
> size of TASK_SIZE_LOW, which is (TASK_SIZE_LOW / PAGE_SIZE / 8).

The bitmap size is determined by CR4.LA57, not the app.  If you place
the bitmap here, won't references to it for high addresses go into the
high address space?

Specifically, on a CR4.LA57=0 system, we have 48 bits of address space,
so 128TB for apps.  You are proposing sticking the bitmap above the
stack which is near the top of that 128TB address space.  But on a
5-level paging system with CR4.LA57=1, there could be valid data at
129GB.  Is there something keeping that data from being mistaken for
being part of the bitmap?

Also, if you're limiting it to TASK_SIZE_LOW, please don't forget that
this is yet another thing that probably won't work with the vsyscall
page.  Please make sure you consider it and mention it in your next post.

> * Mmap the space only when the app issues the first mark-legacy prctl.  This
> avoids the core-dump issue for most apps and the accounting problem that
> MAP_NORESERVE probably won't solve completely.

What is this accounting problem?

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Andy Lutomirski]

> On Jun 14, 2019, at 3:06 PM, Dave Hansen <dave.hansen@intel.com> wrote:
> 
>> On 6/14/19 2:34 PM, Yu-cheng Yu wrote:
>> On Fri, 2019-06-14 at 13:57 -0700, Dave Hansen wrote:
>>>> I have a related question:
>>>> 
>>>> Do we allow the application to read the bitmap, or any fault from the
>>>> application on bitmap pages?
>>> 
>>> We have to allow apps to read it.  Otherwise they can't execute
>>> instructions.
>> 
>> What I meant was, if an app executes some legacy code that results in bitmap
>> lookup, but the bitmap page is not yet populated, and if we then populate that
>> page with all-zero, a #CP should follow.  So do we even populate that zero page
>> at all?
>> 
>> I think we should; a #CP is more obvious to the user at least.
> 
> Please make an effort to un-Intel-ificate your messages as much as
> possible.  I'd really prefer that folks say "missing end branch fault"
> rather than #CP.  I had to Google "#CP".
> 
> I *think* you are saying that:  The *only* lookups to this bitmap are on
> "missing end branch" conditions.  Normal, proper-functioning code
> execution that has ENDBR instructions in it will never even look at the
> bitmap.  The only case when we reference the bitmap locations is when
> the processor is about do do a "missing end branch fault" so that it can
> be suppressed.  Any population with the zero page would be done when
> code had already encountered a "missing end branch" condition, and
> populating with a zero-filled page will guarantee that a "missing end
> branch fault" will result.  You're arguing that we should just figure
> this out at fault time and not ever reach the "missing end branch fault"
> at all.
> 
> Is that right?
> 
> If so, that's an architecture subtlety that I missed until now and which
> went entirely unmentioned in the changelog and discussion up to this
> point.  Let's make sure that nobody else has to walk that path by
> improving our changelog, please.
> 
> In any case, I don't think this is worth special-casing our zero-fill
> code, FWIW.  It's not performance critical and not worth the complexity.
> If apps want to handle the signals and abuse this to fill space up with
> boring page table contents, they're welcome to.  There are much easier
> ways to consume a lot of memory.

Isn’t it a special case either way?  Either we look at CR2 and populate a page, or we look at CR2 and the “tracker” state and send a different signal.  Admittedly the former is very common in the kernel.

> 
>>> We don't have to allow them to (popuating) fault on it.  But, if we
>>> don't, we need some kind of kernel interface to avoid the faults.
>> 
>> The plan is:
>> 
>> * Move STACK_TOP (and vdso) down to give space to the bitmap.
> 
> Even for apps with 57-bit address spaces?
> 
>> * Reserve the bitmap space from (mm->start_stack + PAGE_SIZE) to cover a code
>> size of TASK_SIZE_LOW, which is (TASK_SIZE_LOW / PAGE_SIZE / 8).
> 
> The bitmap size is determined by CR4.LA57, not the app.  If you place
> the bitmap here, won't references to it for high addresses go into the
> high address space?
> 
> Specifically, on a CR4.LA57=0 system, we have 48 bits of address space,
> so 128TB for apps.  You are proposing sticking the bitmap above the
> stack which is near the top of that 128TB address space.  But on a
> 5-level paging system with CR4.LA57=1, there could be valid data at
> 129GB.  Is there something keeping that data from being mistaken for
> being part of the bitmap?
> 

I think we need to make the vma be full sized — it should cover the entire range that the CPU might access. If that means it spans the 48-bit boundary, so be it.

> Also, if you're limiting it to TASK_SIZE_LOW, please don't forget that
> this is yet another thing that probably won't work with the vsyscall
> page.  Please make sure you consider it and mention it in your next post.

Why not?  The vsyscall page is at a negative address.

> 
>> * Mmap the space only when the app issues the first mark-legacy prctl.  This
>> avoids the core-dump issue for most apps and the accounting problem that
>> MAP_NORESERVE probably won't solve

What happens if there’s another VMA there by the time you map it?

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Florian Weimer]

* Dave Hansen:

> My assumption has always been that these large, potentially sparse
> hardware tables *must* be mmap()'d with MAP_NORESERVE specified.  That
> should keep them from being problematic with respect to overcommit.

MAP_NORESERVE pages still count towards the commit limit.  The flag only
disables checks at allocation time, for this particular allocation.  (At
least this was the behavior the last time I looked into this, I
believe.)

Not sure if this makes a difference here.

Thanks,
Florian

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Pavel Machek]

Hi!

> > I've no idea what the kernel should do; since you failed to answer the
> > question what happens when you point this to garbage.
> > 
> > Does it then fault or what?
> 
> Yeah, I think you'll fault with a rather mysterious CR2 value since
> you'll go look at the instruction that faulted and not see any
> references to the CR2 value.
> 
> I think this new MSR probably needs to get included in oops output when
> CET is enabled.
> 
> Why don't we require that a VMA be in place for the entire bitmap?
> Don't we need a "get" prctl function too in case something like a JIT is
> running and needs to find the location of this bitmap to set bits itself?
> 
> Or, do we just go whole-hog and have the kernel manage the bitmap
> itself. Our interface here could be:
> 
> 	prctl(PR_MARK_CODE_AS_LEGACY, start, size);
> 
> and then have the kernel allocate and set the bitmap for those code
> locations.

For the record, that sounds like a better interface than userspace knowing
about the bitmap formats...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Sat, 2019-06-08 at 22:52 +0200, Pavel Machek wrote:
> Hi!
> 
> > > I've no idea what the kernel should do; since you failed to answer the
> > > question what happens when you point this to garbage.
> > > 
> > > Does it then fault or what?
> > 
> > Yeah, I think you'll fault with a rather mysterious CR2 value since
> > you'll go look at the instruction that faulted and not see any
> > references to the CR2 value.
> > 
> > I think this new MSR probably needs to get included in oops output when
> > CET is enabled.
> > 
> > Why don't we require that a VMA be in place for the entire bitmap?
> > Don't we need a "get" prctl function too in case something like a JIT is
> > running and needs to find the location of this bitmap to set bits itself?
> > 
> > Or, do we just go whole-hog and have the kernel manage the bitmap
> > itself. Our interface here could be:
> > 
> > 	prctl(PR_MARK_CODE_AS_LEGACY, start, size);
> > 
> > and then have the kernel allocate and set the bitmap for those code
> > locations.
> 
> For the record, that sounds like a better interface than userspace knowing
> about the bitmap formats...
> 									Pavel

Initially we implemented the bitmap that way.  To manage the bitmap, every time
the application issues a syscall for a .so it loads, and the kernel does
copy_from_user() & copy_to_user() (or similar things).  If a system has a few
legacy .so files and every application does the same, it can take a long time to
boot up.

Yu-cheng

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Pavel Machek]

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

On Mon 2019-06-10 08:47:45, Yu-cheng Yu wrote:
> On Sat, 2019-06-08 at 22:52 +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > > I've no idea what the kernel should do; since you failed to answer the
> > > > question what happens when you point this to garbage.
> > > > 
> > > > Does it then fault or what?
> > > 
> > > Yeah, I think you'll fault with a rather mysterious CR2 value since
> > > you'll go look at the instruction that faulted and not see any
> > > references to the CR2 value.
> > > 
> > > I think this new MSR probably needs to get included in oops output when
> > > CET is enabled.
> > > 
> > > Why don't we require that a VMA be in place for the entire bitmap?
> > > Don't we need a "get" prctl function too in case something like a JIT is
> > > running and needs to find the location of this bitmap to set bits itself?
> > > 
> > > Or, do we just go whole-hog and have the kernel manage the bitmap
> > > itself. Our interface here could be:
> > > 
> > > 	prctl(PR_MARK_CODE_AS_LEGACY, start, size);
> > > 
> > > and then have the kernel allocate and set the bitmap for those code
> > > locations.
> > 
> > For the record, that sounds like a better interface than userspace knowing
> > about the bitmap formats...
> > 									Pavel
> 
> Initially we implemented the bitmap that way.  To manage the bitmap, every time
> the application issues a syscall for a .so it loads, and the kernel does
> copy_from_user() & copy_to_user() (or similar things).  If a system has a few
> legacy .so files and every application does the same, it can take a long time to
> boot up.

Loading .so is already many syscalls, I'd not expect measurable
performance there. Are you sure?
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Dave Hansen]

On 6/6/19 1:09 PM, Yu-cheng Yu wrote:
> +	modify_fpu_regs_begin();
> +	rdmsrl(MSR_IA32_U_CET, r);
> +	r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> +	wrmsrl(MSR_IA32_U_CET, r);
> +	modify_fpu_regs_end();

Isn't there a bunch of other stuff in this MSR?  It seems like the
bitmap value would allow overwriting lots of bits in the MSR that have
nothing to do with the bitmap... in a prctl() that's supposed to only be
dealing with the bitmap.

Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function

[Yu-cheng Yu]

On Fri, 2019-06-07 at 12:03 -0700, Dave Hansen wrote:
> On 6/6/19 1:09 PM, Yu-cheng Yu wrote:
> > +	modify_fpu_regs_begin();
> > +	rdmsrl(MSR_IA32_U_CET, r);
> > +	r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> > +	wrmsrl(MSR_IA32_U_CET, r);
> > +	modify_fpu_regs_end();
> 
> Isn't there a bunch of other stuff in this MSR?  It seems like the
> bitmap value would allow overwriting lots of bits in the MSR that have
> nothing to do with the bitmap... in a prctl() that's supposed to only be
> dealing with the bitmap.

Yes, the bitmap address should have been masked, although it is checked for page
alignment (which has the same effect).  I will fix it.

Yu-cheng

[PATCH v7 04/14] x86/cet/ibt: Handle signals for IBT

[Yu-cheng Yu]

Setup/Restore Indirect Branch Tracking for signals.

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

diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
index e0ef996d3148..e1ab7e722637 100644
--- a/arch/x86/kernel/cet.c
+++ b/arch/x86/kernel/cet.c
@@ -282,6 +282,15 @@ int cet_restore_signal(bool ia32, struct sc_ext *sc_ext)
 		msr_ia32_u_cet |= MSR_IA32_CET_SHSTK_EN;
 	}
 
+	if (current->thread.cet.ibt_enabled) {
+		if (current->thread.cet.ibt_bitmap_addr != 0)
+			msr_ia32_u_cet |= (current->thread.cet.ibt_bitmap_addr |
+					   MSR_IA32_CET_LEG_IW_EN);
+
+		msr_ia32_u_cet |= (MSR_IA32_CET_ENDBR_EN |
+				   MSR_IA32_CET_NO_TRACK_EN);
+	}
+
 	wrmsrl(MSR_IA32_PL3_SSP, new_ssp);
 	wrmsrl(MSR_IA32_U_CET, msr_ia32_u_cet);
 	return 0;
@@ -322,6 +331,15 @@ int cet_setup_signal(bool ia32, unsigned long rstor_addr, struct sc_ext *sc_ext)
 		sc_ext->ssp = new_ssp;
 	}
 
+	if (current->thread.cet.ibt_enabled) {
+		if (current->thread.cet.ibt_bitmap_addr != 0)
+			msr_ia32_u_cet |= (current->thread.cet.ibt_bitmap_addr |
+					   MSR_IA32_CET_LEG_IW_EN);
+
+		msr_ia32_u_cet |= (MSR_IA32_CET_ENDBR_EN |
+				   MSR_IA32_CET_NO_TRACK_EN);
+	}
+
 	modify_fpu_regs_begin();
 	wrmsrl(MSR_IA32_PL3_SSP, ssp);
 	wrmsrl(MSR_IA32_U_CET, msr_ia32_u_cet);
-- 
2.17.1

[PATCH v7 05/14] 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/asm/mmu_context.h | 10 ++++++++++
 mm/mmap.c                          | 19 ++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index a9a768529540..2499f6490428 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -360,6 +360,16 @@ static inline unsigned long __get_current_cr3_fast(void)
 	return cr3;
 }
 
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+static inline unsigned long arch_as_limit(void)
+{
+	if (current->thread.cet.ibt_enabled)
+		return current->thread.cet.ibt_bitmap_size;
+	else
+		return 0;
+}
+#endif
+
 typedef struct {
 	struct mm_struct *mm;
 } temp_mm_state_t;
diff --git a/mm/mmap.c b/mm/mmap.c
index 3b643ace2c49..a0d6fb559518 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3283,13 +3283,30 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
 	return NULL;
 }
 
+#ifndef CONFIG_ARCH_HAS_AS_LIMIT
+static inline unsigned long arch_as_limit(void)
+{
+	return 0;
+}
+#endif
+
 /*
  * Return true if the calling process may expand its vm space by the passed
  * number of pages
  */
 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 + arch_as_limit();
+
+	/* 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

[PATCH v7 06/14] 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: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/kernel/process_64.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 5fa0d9ab18f1..16dae646f633 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -856,6 +856,12 @@ int arch_setup_property(void *ehdr, void *phdr, struct file *f, bool inter)
 		if (r < 0)
 			return r;
 	}
+
+	if (cpu_feature_enabled(X86_FEATURE_IBT)) {
+		if (property & GNU_PROPERTY_X86_FEATURE_1_IBT)
+			r = cet_setup_ibt();
+	}
+
 	return r;
 }
 #endif
-- 
2.17.1

[PATCH v7 07/14] x86/cet/ibt: Add arch_prctl functions for IBT

[Yu-cheng Yu]

Update ARCH_X86_CET_STATUS and ARCH_X86_CET_DISABLE to include
Indirect Branch Tracking features.

Introduce:

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

    The parameter 'addr' is a pointer to a user buffer that has:

    *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 |  2 ++
 arch/x86/kernel/cet_prctl.c       | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h
index d962f0ec9ccf..5eb9aeb5c662 100644
--- a/arch/x86/include/uapi/asm/prctl.h
+++ b/arch/x86/include/uapi/asm/prctl.h
@@ -18,5 +18,7 @@
 #define ARCH_X86_CET_DISABLE		0x3002
 #define ARCH_X86_CET_LOCK		0x3003
 #define ARCH_X86_CET_ALLOC_SHSTK	0x3004
+#define ARCH_X86_CET_GET_LEGACY_BITMAP	0x3005 /* deprecated */
+#define ARCH_X86_CET_SET_LEGACY_BITMAP	0x3006
 
 #endif /* _ASM_X86_PRCTL_H */
diff --git a/arch/x86/kernel/cet_prctl.c b/arch/x86/kernel/cet_prctl.c
index 9c9d4262b07e..b7f37bbc0dd3 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;
@@ -55,6 +57,17 @@ static int handle_alloc_shstk(unsigned long arg2)
 	return 0;
 }
 
+static int handle_bitmap(unsigned long arg2)
+{
+	unsigned long addr, size;
+
+	if (get_user(addr, (unsigned long __user *)arg2) ||
+	    get_user(size, (unsigned long __user *)arg2 + 1))
+		return -EFAULT;
+
+	return cet_setup_ibt_bitmap(addr, size);
+}
+
 int prctl_cet(int option, unsigned long arg2)
 {
 	if (!cpu_x86_cet_enabled())
@@ -69,6 +82,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;
 
@@ -79,6 +94,12 @@ int prctl_cet(int option, unsigned long arg2)
 	case ARCH_X86_CET_ALLOC_SHSTK:
 		return handle_alloc_shstk(arg2);
 
+	/*
+	 * Allocate legacy bitmap and return address & size to user.
+	 */
+	case ARCH_X86_CET_SET_LEGACY_BITMAP:
+		return handle_bitmap(arg2);
+
 	default:
 		return -EINVAL;
 	}
-- 
2.17.1

Re: [PATCH v7 07/14] x86/cet/ibt: Add arch_prctl functions for IBT

[Peter Zijlstra]

On Thu, Jun 06, 2019 at 01:09:19PM -0700, Yu-cheng Yu wrote:

> +static int handle_bitmap(unsigned long arg2)
> +{
> +	unsigned long addr, size;
> +
> +	if (get_user(addr, (unsigned long __user *)arg2) ||
> +	    get_user(size, (unsigned long __user *)arg2 + 1))
> +		return -EFAULT;
> +
> +	return cet_setup_ibt_bitmap(addr, size);
> +}


> +	/*
> +	 * Allocate legacy bitmap and return address & size to user.
> +	 */
> +	case ARCH_X86_CET_SET_LEGACY_BITMAP:
> +		return handle_bitmap(arg2);

AFAICT it does exactly the opposite of that comment; it gets the address
and size from userspace and doesn't allocate anything at all.

[PATCH v7 08/14] 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

[PATCH v7 09/14] x86/vdso: 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/Makefile          | 12 +++++++++++-
 arch/x86/entry/vdso/vdso-layout.lds.S |  1 +
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 42fe42e82baf..718fc17b0d67 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -108,13 +108,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 FORCE
 	$(call if_changed,objcopy)
@@ -173,6 +177,12 @@ quiet_cmd_vdso = VDSO    $@
 VDSO_LDFLAGS = -shared $(call ld-option, --hash-style=both) \
 	$(call ld-option, --build-id) $(call ld-option, --eh-frame-hdr) \
 	-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 93c6dc7812d0..3fea2ce318bc 100644
--- a/arch/x86/entry/vdso/vdso-layout.lds.S
+++ b/arch/x86/entry/vdso/vdso-layout.lds.S
@@ -52,6 +52,7 @@ SECTIONS
 		*(.gnu.linkonce.b.*)
 	}						: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

Re: [PATCH v7 09/14] x86/vdso: Insert endbr32/endbr64 to vDSO

[Andy Lutomirski]

On Thu, Jun 6, 2019 at 1:17 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> 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.

Acked-by: Andy Lutomirski <luto@kernel.org>

>
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>

You're still missing your Signed-off-by.

[PATCH v7 10/14] x86/vdso/32: Add ENDBR32 to __kernel_vsyscall entry point

[Yu-cheng Yu]

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

Add ENDBR32 to __kernel_vsyscall entry point.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 arch/x86/entry/vdso/vdso32/system_call.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/entry/vdso/vdso32/system_call.S b/arch/x86/entry/vdso/vdso32/system_call.S
index 263d7433dea8..2fc8141fff4e 100644
--- a/arch/x86/entry/vdso/vdso32/system_call.S
+++ b/arch/x86/entry/vdso/vdso32/system_call.S
@@ -14,6 +14,9 @@
 	ALIGN
 __kernel_vsyscall:
 	CFI_STARTPROC
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+	endbr32
+#endif
 	/*
 	 * Reshuffle regs so that all of any of the entry instructions
 	 * will preserve enough state.
-- 
2.17.1

Re: [PATCH v7 10/14] x86/vdso/32: Add ENDBR32 to __kernel_vsyscall entry point

[Andy Lutomirski]

On Thu, Jun 6, 2019 at 1:17 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> From: "H.J. Lu" <hjl.tools@gmail.com>
>
> Add ENDBR32 to __kernel_vsyscall entry point.
>

Acked-by: Andy Lutomirski <luto@kernel.org>

However, you forgot your own Signed-off-by.

> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>


--Andy

[PATCH v7 11/14] x86/vsyscall/64: Add ENDBR64 to vsyscall entry points

[Yu-cheng Yu]

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

Add ENDBR64 to vsyscall entry points.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Acked-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/vsyscall/vsyscall_emu_64.S | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/entry/vsyscall/vsyscall_emu_64.S b/arch/x86/entry/vsyscall/vsyscall_emu_64.S
index 2e203f3a25a7..040696333457 100644
--- a/arch/x86/entry/vsyscall/vsyscall_emu_64.S
+++ b/arch/x86/entry/vsyscall/vsyscall_emu_64.S
@@ -17,16 +17,25 @@ __PAGE_ALIGNED_DATA
 	.type __vsyscall_page, @object
 __vsyscall_page:
 
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+	endbr64
+#endif
 	mov $__NR_gettimeofday, %rax
 	syscall
 	ret
 
 	.balign 1024, 0xcc
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+	endbr64
+#endif
 	mov $__NR_time, %rax
 	syscall
 	ret
 
 	.balign 1024, 0xcc
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+	endbr64
+#endif
 	mov $__NR_getcpu, %rax
 	syscall
 	ret
-- 
2.17.1

Re: [PATCH v7 11/14] x86/vsyscall/64: Add ENDBR64 to vsyscall entry points

[Andy Lutomirski]

On Thu, Jun 6, 2019 at 1:17 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> From: "H.J. Lu" <hjl.tools@gmail.com>
>
> Add ENDBR64 to vsyscall entry points.

I'm still okay with this patch, but this is rather silly.  If anyone
actually executes this code, they're doing it wrong.

--Andy

[PATCH v7 12/14] x86/vsyscall/64: Fixup shadow stack and branch tracking for vsyscall

[Yu-cheng Yu]

When emulating a RET, also unwind the task's shadow stack and cancel
the current branch tracking status.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/entry/vsyscall/vsyscall_64.c | 28 +++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
index d9d81ad7a400..6869ef9d1e8b 100644
--- a/arch/x86/entry/vsyscall/vsyscall_64.c
+++ b/arch/x86/entry/vsyscall/vsyscall_64.c
@@ -38,6 +38,8 @@
 #include <asm/fixmap.h>
 #include <asm/traps.h>
 #include <asm/paravirt.h>
+#include <asm/fpu/xstate.h>
+#include <asm/fpu/types.h>
 
 #define CREATE_TRACE_POINTS
 #include "vsyscall_trace.h"
@@ -92,6 +94,30 @@ static int addr_to_vsyscall_nr(unsigned long addr)
 	return nr;
 }
 
+void fixup_shstk(void)
+{
+#ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
+	u64 r;
+
+	if (current->thread.cet.shstk_enabled) {
+		rdmsrl(MSR_IA32_PL3_SSP, r);
+		wrmsrl(MSR_IA32_PL3_SSP, r + 8);
+	}
+#endif
+}
+
+void fixup_ibt(void)
+{
+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+	u64 r;
+
+	if (current->thread.cet.ibt_enabled) {
+		rdmsrl(MSR_IA32_U_CET, r);
+		wrmsrl(MSR_IA32_U_CET, r & ~MSR_IA32_CET_WAIT_ENDBR);
+	}
+#endif
+}
+
 static bool write_ok_or_segv(unsigned long ptr, size_t size)
 {
 	/*
@@ -265,6 +291,8 @@ bool emulate_vsyscall(struct pt_regs *regs, unsigned long address)
 	/* Emulate a ret instruction. */
 	regs->ip = caller;
 	regs->sp += 8;
+	fixup_shstk();
+	fixup_ibt();
 	return true;
 
 sigsegv:
-- 
2.17.1

Re: [PATCH v7 12/14] x86/vsyscall/64: Fixup shadow stack and branch tracking for vsyscall

[Andy Lutomirski]

On Thu, Jun 6, 2019 at 1:17 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> When emulating a RET, also unwind the task's shadow stack and cancel
> the current branch tracking status.
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
>  arch/x86/entry/vsyscall/vsyscall_64.c | 28 +++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
> index d9d81ad7a400..6869ef9d1e8b 100644
> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
> @@ -38,6 +38,8 @@
>  #include <asm/fixmap.h>
>  #include <asm/traps.h>
>  #include <asm/paravirt.h>
> +#include <asm/fpu/xstate.h>
> +#include <asm/fpu/types.h>
>
>  #define CREATE_TRACE_POINTS
>  #include "vsyscall_trace.h"
> @@ -92,6 +94,30 @@ static int addr_to_vsyscall_nr(unsigned long addr)
>         return nr;
>  }
>
> +void fixup_shstk(void)
> +{
> +#ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
> +       u64 r;
> +
> +       if (current->thread.cet.shstk_enabled) {
> +               rdmsrl(MSR_IA32_PL3_SSP, r);
> +               wrmsrl(MSR_IA32_PL3_SSP, r + 8);
> +       }
> +#endif
> +}
> +
> +void fixup_ibt(void)
> +{
> +#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
> +       u64 r;
> +
> +       if (current->thread.cet.ibt_enabled) {
> +               rdmsrl(MSR_IA32_U_CET, r);
> +               wrmsrl(MSR_IA32_U_CET, r & ~MSR_IA32_CET_WAIT_ENDBR);
> +       }
> +#endif
> +}

These should be static.

But please just inline them directly in their one call site.  The code
will be a lot easier to understand.

--Andy

[PATCH v7 13/14] 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) and
    IA32_PL3_SSP (user-mode 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 d652b939ccfb..2937ec9d9215 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -156,6 +156,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_CET_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_CET_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 a166c960bc9e..db902ed9b353 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -51,7 +51,9 @@ enum x86_regset {
 	REGSET_IOPERM64 = REGSET_XFP,
 	REGSET_XSTATE,
 	REGSET_TLS,
+	REGSET_CET64 = REGSET_TLS,
 	REGSET_IOPERM32,
+	REGSET_CET32,
 };
 
 struct pt_regs_offset {
@@ -1268,6 +1270,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 = {
@@ -1323,6 +1332,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 316177ce9e76..4f320d96d538 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

[PATCH v7 14/14] x86: Discard .note.gnu.property sections

[Yu-cheng Yu]

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

With the command-line option, -mx86-used-note=yes, the x86 assembler
in binutils 2.32 and above generates a program property note in a note
section, .note.gnu.property, to encode used x86 ISAs and features.
To exclude .note.gnu.property sections from NOTE segment in x86 kernel
linker script:

PHDRS {
 text PT_LOAD FLAGS(5);
 data PT_LOAD FLAGS(6);
 percpu PT_LOAD FLAGS(6);
 init PT_LOAD FLAGS(7);
 note PT_NOTE FLAGS(0);
}
SECTIONS
{
...
 .notes : AT(ADDR(.notes) - 0xffffffff80000000) { __start_notes = .; KEEP(*(.not
e.*)) __stop_notes = .; } :text :note
...
}

this patch discards .note.gnu.property sections in kernel linker script
by adding

 /DISCARD/ : {
  *(.note.gnu.property)
 }

before .notes sections.  Since .exit.text and .exit.data sections are
discarded at runtime, it undefines EXIT_TEXT and EXIT_DATA to exclude
.exit.text and .exit.data sections from default discarded sections.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 arch/x86/kernel/vmlinux.lds.S | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 0850b5149345..d2594b482c09 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -146,6 +146,10 @@ SECTIONS
 	/* End of text section */
 	_etext = .;
 
+	/* .note.gnu.property sections should be discarded */
+	/DISCARD/ : {
+		*(.note.gnu.property)
+	}
 	NOTES :text :note
 
 	EXCEPTION_TABLE(16) :text = 0x9090
@@ -382,7 +386,12 @@ SECTIONS
 	STABS_DEBUG
 	DWARF_DEBUG
 
-	/* Sections to be discarded */
+	/* Sections to be discarded.  EXIT_TEXT and EXIT_DATA discard at runtime.
+	 * not link time.  */
+#undef EXIT_TEXT
+#define EXIT_TEXT
+#undef EXIT_DATA
+#define EXIT_DATA
 	DISCARDS
 	/DISCARD/ : {
 		*(.eh_frame)
-- 
2.17.1