2014-01-22 02:14:47

by Ren Qiaowei

[permalink] [raw]
Subject: [PATCH v2 0/4] Intel MPX support

Changes since v1:
* check to see if #BR occurred in userspace or kernel space.
* use generic structure and macro as much as possible when
decode mpx instructions.

Qiaowei Ren (4):
x86, mpx: add documentation on Intel MPX
x86, mpx: hook #BR exception handler to allocate bound tables
x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
x86, mpx: extend siginfo structure to include bound violation
information

Documentation/x86/intel_mpx.txt | 76 +++++++
arch/x86/Kconfig | 4 +
arch/x86/include/asm/mpx.h | 63 ++++++
arch/x86/include/asm/processor.h | 16 ++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/mpx.c | 417 ++++++++++++++++++++++++++++++++++++
arch/x86/kernel/traps.c | 61 +++++-
include/uapi/asm-generic/siginfo.h | 9 +-
include/uapi/linux/prctl.h | 6 +
kernel/signal.c | 4 +
kernel/sys.c | 12 +
11 files changed, 667 insertions(+), 2 deletions(-)
create mode 100644 Documentation/x86/intel_mpx.txt
create mode 100644 arch/x86/include/asm/mpx.h
create mode 100644 arch/x86/kernel/mpx.c


2014-01-22 02:14:49

by Ren Qiaowei

[permalink] [raw]
Subject: [PATCH v2 1/4] x86, mpx: add documentation on Intel MPX

This patch adds the Documentation/x86/intel_mpx.txt file with some
information about Intel MPX.

Signed-off-by: Qiaowei Ren <[email protected]>
---
Documentation/x86/intel_mpx.txt | 76 +++++++++++++++++++++++++++++++++++++++
1 files changed, 76 insertions(+), 0 deletions(-)
create mode 100644 Documentation/x86/intel_mpx.txt

diff --git a/Documentation/x86/intel_mpx.txt b/Documentation/x86/intel_mpx.txt
new file mode 100644
index 0000000..778d06e
--- /dev/null
+++ b/Documentation/x86/intel_mpx.txt
@@ -0,0 +1,76 @@
+Intel(R) MPX Overview:
+=====================
+
+Intel(R) Memory Protection Extensions (Intel(R) MPX) is a new
+capability introduced into Intel Architecture. Intel MPX can
+increase the robustness of software when it is used in conjunction
+with compiler changes to check that memory references intended
+at compile time do not become unsafe at runtime.
+
+Two of the most important goals of Intel MPX are to provide
+this capability at very low performance overhead for newly
+compiled code, and to provide compatibility mechanisms with
+legacy software components. A direct benefit Intel MPX provides
+is hardening software against malicious attacks designed to
+cause or exploit buffer overruns.
+
+For details about the Intel MPX instructions, see "Intel(R)
+Architecture Instruction Set Extensions Programming Reference".
+
+Intel(R) MPX Programming Model
+------------------------------
+
+Intel MPX introduces new registers and new instructions that
+operate on these registers. Some of the registers added are
+bounds registers which store a pointer's lower bound and upper
+bound limits. Whenever the pointer is used, the requested
+reference is checked against the pointer's associated bounds,
+thereby preventing out-of-bound memory access (such as buffer
+overflows and overruns). Out-of-bounds memory references
+initiate a #BR exception which can then be handled in an
+appropriate manner.
+
+Loading and Storing Bounds using Translation
+--------------------------------------------
+
+Intel MPX defines two instructions for load/store of the linear
+address of a pointer to a buffer, along with the bounds of the
+buffer into a paging structure of extended bounds. Specifically
+when storing extended bounds, the processor will perform address
+translation of the address where the pointer is stored to an
+address in the Bound Table (BT) to determine the store location
+of extended bounds. Loading of an extended bounds performs the
+reverse sequence.
+
+The structure in memory to load/store an extended bound is a
+4-tuple consisting of lower bound, upper bound, pointer value
+and a reserved field. Bound loads and stores access 32-bit or
+64-bit operand size according to the operation mode. Thus,
+a bound table entry is 4*32 bits in 32-bit mode and 4*64 bits
+in 64-bit mode.
+
+The linear address of a bound table is stored in a Bound
+Directory (BD) entry. And the linear address of the bound
+directory is derived from either BNDCFGU or BNDCFGS registers.
+Bounds in memory are stored in Bound Tables (BT) as an extended
+bound, which are accessed via Bound Directory (BD) and address
+translation performed by BNDLDX/BNDSTX instructions.
+
+Bounds Directory (BD) and Bounds Tables (BT) are stored in
+application memory and are allocated by the application (in case
+of kernel use, the structures will be in kernel memory). The
+bound directory and each instance of bound table are in contiguous
+linear memory.
+
+XSAVE/XRESTOR Support of Intel MPX State
+----------------------------------------
+
+Enabling Intel MPX requires an OS to manage two bits in XCR0:
+ - BNDREGS for saving and restoring registers BND0-BND3,
+ - BNDCSR for saving and restoring the user-mode configuration
+(BNDCFGU) and the status register (BNDSTATUS).
+
+The reason for having two separate bits is that BND0-BND3 is
+likely to be volatile state, while BNDCFGU and BNDSTATUS are not.
+Therefore, an OS has flexibility in handling these two states
+differently in saving or restoring them.
--
1.7.1

2014-01-22 02:15:00

by Ren Qiaowei

[permalink] [raw]
Subject: [PATCH v2 3/4] x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE

This patch adds the PR_MPX_INIT and PR_MPX_RELEASE prctl()
commands on the x86 platform. These commands can be used to
init and release MPX related resource.

A MMU notifier will be registered during PR_MPX_INIT
command execution. So the bound tables can be automatically
deallocated when one memory area is unmapped.

Signed-off-by: Qiaowei Ren <[email protected]>
---
arch/x86/Kconfig | 4 ++
arch/x86/include/asm/mpx.h | 9 ++++
arch/x86/include/asm/processor.h | 16 +++++++
arch/x86/kernel/mpx.c | 84 ++++++++++++++++++++++++++++++++++++++
include/uapi/linux/prctl.h | 6 +++
kernel/sys.c | 12 +++++
6 files changed, 131 insertions(+), 0 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ee2fb9d..695101a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -233,6 +233,10 @@ config HAVE_INTEL_TXT
def_bool y
depends on INTEL_IOMMU && ACPI

+config HAVE_INTEL_MPX
+ def_bool y
+ select MMU_NOTIFIER
+
config X86_32_SMP
def_bool y
depends on X86_32 && SMP
diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
index d074153..9652e9e 100644
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -30,6 +30,15 @@

#endif

+typedef union {
+ struct {
+ unsigned long ignored:MPX_IGN_BITS;
+ unsigned long l2index:MPX_L2_BITS;
+ unsigned long l1index:MPX_L1_BITS;
+ };
+ unsigned long addr;
+} mpx_addr;
+
void do_mpx_bt_fault(struct xsave_struct *xsave_buf);

#endif /* _ASM_X86_MPX_H */
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 43be6f6..ea4e72d 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -963,6 +963,22 @@ extern void start_thread(struct pt_regs *regs, unsigned long new_ip,
extern int get_tsc_mode(unsigned long adr);
extern int set_tsc_mode(unsigned int val);

+#ifdef CONFIG_HAVE_INTEL_MPX
+
+/* Init/release a process' MPX related resource */
+#define MPX_INIT(tsk) mpx_init((tsk))
+#define MPX_RELEASE(tsk) mpx_release((tsk))
+
+extern int mpx_init(struct task_struct *tsk);
+extern int mpx_release(struct task_struct *tsk);
+
+#else /* CONFIG_HAVE_INTEL_MPX */
+
+#define MPX_INIT(tsk) (-EINVAL)
+#define MPX_RELEASE(tsk) (-EINVAL)
+
+#endif /* CONFIG_HAVE_INTEL_MPX */
+
extern u16 amd_get_nb_id(int cpu);

static inline uint32_t hypervisor_cpuid_base(const char *sig, uint32_t leaves)
diff --git a/arch/x86/kernel/mpx.c b/arch/x86/kernel/mpx.c
index 767b3bf..ffe5aee 100644
--- a/arch/x86/kernel/mpx.c
+++ b/arch/x86/kernel/mpx.c
@@ -1,5 +1,7 @@
#include <linux/kernel.h>
#include <linux/syscalls.h>
+#include <linux/prctl.h>
+#include <linux/mmu_notifier.h>
#include <asm/processor.h>
#include <asm/mpx.h>
#include <asm/mman.h>
@@ -7,6 +9,88 @@
#include <asm/fpu-internal.h>
#include <asm/alternative.h>

+static struct mmu_notifier mpx_mn;
+
+static void mpx_invl_range_end(struct mmu_notifier *mn,
+ struct mm_struct *mm,
+ unsigned long start, unsigned long end)
+{
+ struct xsave_struct *xsave_buf;
+ unsigned long bt_size = 1UL << (MPX_L2_BITS+MPX_L2_SHIFT);
+ unsigned long bt_addr;
+ unsigned long bd_base;
+ unsigned long bd_entry, bde_start, bde_end;
+ mpx_addr lap;
+
+ pgd_t *pgd;
+ pud_t *pud;
+ pmd_t *pmd;
+ pte_t *pte;
+
+ /* ignore swap notifications */
+ pgd = pgd_offset(mm, start);
+ pud = pud_offset(pgd, start);
+ pmd = pmd_offset(pud, start);
+ pte = pte_offset_kernel(pmd, start);
+ if (!pte_present(*pte) && !pte_none(*pte) && !pte_file(*pte))
+ return;
+
+ /* get bound directory base address */
+ fpu_xsave(&current->thread.fpu);
+ xsave_buf = &(current->thread.fpu.state->xsave);
+ bd_base = xsave_buf->bndcsr.cfg_reg_u & MPX_BNDCFG_ADDR_MASK;
+
+ /* get related bde range */
+ lap.addr = start;
+ bde_start = bd_base + (lap.l1index << MPX_L1_SHIFT);
+
+ lap.addr = end;
+ if (lap.ignored || lap.l2index)
+ bde_end = bd_base + (lap.l1index<<MPX_L1_SHIFT) + sizeof(long);
+ else
+ bde_end = bd_base + (lap.l1index<<MPX_L1_SHIFT);
+
+ for (bd_entry = bde_start; bd_entry < bde_end;
+ bd_entry += sizeof(unsigned long)) {
+
+ if (get_user(bt_addr, (long __user *)bd_entry))
+ return;
+
+ if (bt_addr & 0x1) {
+ user_atomic_cmpxchg_inatomic(&bt_addr,
+ (long __user *)bd_entry, bt_addr, 0);
+ do_munmap(mm, bt_addr & MPX_L2_NODE_ADDR_MASK, bt_size);
+ }
+ }
+}
+
+static struct mmu_notifier_ops mpx_mmuops = {
+ .invalidate_range_end = mpx_invl_range_end,
+};
+
+int mpx_init(struct task_struct *tsk)
+{
+ if (!boot_cpu_has(X86_FEATURE_MPX))
+ return -EINVAL;
+
+ /* register mmu_notifier to delallocate the bound tables */
+ mpx_mn.ops = &mpx_mmuops;
+ mmu_notifier_register(&mpx_mn, current->mm);
+
+ return 0;
+}
+
+int mpx_release(struct task_struct *tsk)
+{
+ if (!boot_cpu_has(X86_FEATURE_MPX))
+ return -EINVAL;
+
+ /* unregister mmu_notifier */
+ mmu_notifier_unregister(&mpx_mn, current->mm);
+
+ return 0;
+}
+
static bool allocate_bt(unsigned long bd_entry)
{
unsigned long bt_size = 1UL << (MPX_L2_BITS+MPX_L2_SHIFT);
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 289760f..19ab881 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -149,4 +149,10 @@

#define PR_GET_TID_ADDRESS 40

+/*
+ * Init/release MPX related resource.
+ */
+#define PR_MPX_INIT 41
+#define PR_MPX_RELEASE 42
+
#endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index c18ecca..bbaf573 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -92,6 +92,12 @@
#ifndef SET_TSC_CTL
# define SET_TSC_CTL(a) (-EINVAL)
#endif
+#ifndef MPX_INIT
+# define MPX_INIT(a) (-EINVAL)
+#endif
+#ifndef MPX_RELEASE
+# define MPX_RELEASE(a) (-EINVAL)
+#endif

/*
* this is where the system-wide overflow UID and GID are defined, for
@@ -1999,6 +2005,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
if (arg2 || arg3 || arg4 || arg5)
return -EINVAL;
return current->no_new_privs ? 1 : 0;
+ case PR_MPX_INIT:
+ error = MPX_INIT(me);
+ break;
+ case PR_MPX_RELEASE:
+ error = MPX_RELEASE(me);
+ break;
default:
error = -EINVAL;
break;
--
1.7.1

2014-01-22 02:14:55

by Ren Qiaowei

[permalink] [raw]
Subject: [PATCH v2 2/4] x86, mpx: hook #BR exception handler to allocate bound tables

An access to an invalid bound directory entry will cause a #BR
exception. This patch hook #BR exception handler to allocate
one bound table and bind it with that buond directory entry.

This will avoid the need of forwarding the #BR exception
to the user space when bound directory has invalid entry.

Signed-off-by: Qiaowei Ren <[email protected]>
---
arch/x86/include/asm/mpx.h | 35 ++++++++++++++++++++++++++++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/mpx.c | 44 +++++++++++++++++++++++++++++++++++
arch/x86/kernel/traps.c | 55 +++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 134 insertions(+), 1 deletions(-)
create mode 100644 arch/x86/include/asm/mpx.h
create mode 100644 arch/x86/kernel/mpx.c

diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
new file mode 100644
index 0000000..d074153
--- /dev/null
+++ b/arch/x86/include/asm/mpx.h
@@ -0,0 +1,35 @@
+#ifndef _ASM_X86_MPX_H
+#define _ASM_X86_MPX_H
+
+#include <linux/types.h>
+#include <asm/ptrace.h>
+
+#ifdef CONFIG_X86_64
+
+#define MPX_L1_BITS 28
+#define MPX_L1_SHIFT 3
+#define MPX_L2_BITS 17
+#define MPX_L2_SHIFT 5
+#define MPX_IGN_BITS 3
+#define MPX_L2_NODE_ADDR_MASK 0xfffffffffffffff8UL
+
+#define MPX_BNDSTA_ADDR_MASK 0xfffffffffffffffcUL
+#define MPX_BNDCFG_ADDR_MASK 0xfffffffffffff000UL
+
+#else
+
+#define MPX_L1_BITS 20
+#define MPX_L1_SHIFT 2
+#define MPX_L2_BITS 10
+#define MPX_L2_SHIFT 4
+#define MPX_IGN_BITS 2
+#define MPX_L2_NODE_ADDR_MASK 0xfffffffcUL
+
+#define MPX_BNDSTA_ADDR_MASK 0xfffffffcUL
+#define MPX_BNDCFG_ADDR_MASK 0xfffff000UL
+
+#endif
+
+void do_mpx_bt_fault(struct xsave_struct *xsave_buf);
+
+#endif /* _ASM_X86_MPX_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index a5408b9..bba7a71 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -38,6 +38,7 @@ obj-y += resource.o

obj-y += process.o
obj-y += i387.o xsave.o
+obj-y += mpx.o
obj-y += ptrace.o
obj-$(CONFIG_X86_32) += tls.o
obj-$(CONFIG_IA32_EMULATION) += tls.o
diff --git a/arch/x86/kernel/mpx.c b/arch/x86/kernel/mpx.c
new file mode 100644
index 0000000..767b3bf
--- /dev/null
+++ b/arch/x86/kernel/mpx.c
@@ -0,0 +1,44 @@
+#include <linux/kernel.h>
+#include <linux/syscalls.h>
+#include <asm/processor.h>
+#include <asm/mpx.h>
+#include <asm/mman.h>
+#include <asm/i387.h>
+#include <asm/fpu-internal.h>
+#include <asm/alternative.h>
+
+static bool allocate_bt(unsigned long bd_entry)
+{
+ unsigned long bt_size = 1UL << (MPX_L2_BITS+MPX_L2_SHIFT);
+ unsigned long bt_addr, old_val;
+
+ bt_addr = sys_mmap_pgoff(0, bt_size, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE | MAP_POPULATE, -1, 0);
+ if (bt_addr == -1) {
+ pr_err("L2 Node Allocation Failed at L1 addr %lx\n",
+ bd_entry);
+ return false;
+ }
+ bt_addr = (bt_addr & MPX_L2_NODE_ADDR_MASK) | 0x01;
+
+ user_atomic_cmpxchg_inatomic(&old_val,
+ (long __user *)bd_entry, 0, bt_addr);
+ if (old_val)
+ vm_munmap(bt_addr & MPX_L2_NODE_ADDR_MASK, bt_size);
+
+ return true;
+}
+
+void do_mpx_bt_fault(struct xsave_struct *xsave_buf)
+{
+ unsigned long status;
+ unsigned long bd_entry, bd_base;
+ unsigned long bd_size = 1UL << (MPX_L1_BITS+MPX_L1_SHIFT);
+
+ bd_base = xsave_buf->bndcsr.cfg_reg_u & MPX_BNDCFG_ADDR_MASK;
+ status = xsave_buf->bndcsr.status_reg;
+
+ bd_entry = status & MPX_BNDSTA_ADDR_MASK;
+ if ((bd_entry >= bd_base) && (bd_entry < bd_base + bd_size))
+ allocate_bt(bd_entry);
+}
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 8c8093b..78e9c16 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -59,6 +59,7 @@
#include <asm/fixmap.h>
#include <asm/mach_traps.h>
#include <asm/alternative.h>
+#include <asm/mpx.h>

#ifdef CONFIG_X86_64
#include <asm/x86_init.h>
@@ -214,7 +215,6 @@ dotraplinkage void do_##name(struct pt_regs *regs, long error_code) \
DO_ERROR_INFO(X86_TRAP_DE, SIGFPE, "divide error", divide_error, FPE_INTDIV,
regs->ip)
DO_ERROR(X86_TRAP_OF, SIGSEGV, "overflow", overflow)
-DO_ERROR(X86_TRAP_BR, SIGSEGV, "bounds", bounds)
DO_ERROR_INFO(X86_TRAP_UD, SIGILL, "invalid opcode", invalid_op, ILL_ILLOPN,
regs->ip)
DO_ERROR(X86_TRAP_OLD_MF, SIGFPE, "coprocessor segment overrun",
@@ -267,6 +267,59 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code)
}
#endif

+dotraplinkage void do_bounds(struct pt_regs *regs, long error_code)
+{
+ enum ctx_state prev_state;
+ unsigned long status;
+ struct xsave_struct *xsave_buf;
+ struct task_struct *tsk = current;
+
+ prev_state = exception_enter();
+ if (notify_die(DIE_TRAP, "bounds", regs, error_code,
+ X86_TRAP_BR, SIGSEGV) == NOTIFY_STOP)
+ goto exit;
+ conditional_sti(regs);
+
+ if (!user_mode(regs)) {
+ if (!fixup_exception(regs)) {
+ tsk->thread.error_code = error_code;
+ tsk->thread.trap_nr = X86_TRAP_BR;
+ die("bounds", regs, error_code);
+ }
+ goto exit;
+ }
+
+ if (!boot_cpu_has(X86_FEATURE_MPX)) {
+ do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs, error_code, NULL);
+ goto exit;
+ }
+
+ fpu_xsave(&tsk->thread.fpu);
+ xsave_buf = &(tsk->thread.fpu.state->xsave);
+ status = xsave_buf->bndcsr.status_reg;
+
+ switch (status & 0x3) {
+ case 2:
+ /*
+ * Bound directory has invalid entry.
+ * No signal will be sent to the user space.
+ */
+ do_mpx_bt_fault(xsave_buf);
+ break;
+
+ case 1: /* Bound violation. */
+ case 0: /* No MPX exception. */
+ do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs, error_code, NULL);
+ break;
+
+ default:
+ break;
+ }
+
+exit:
+ exception_exit(prev_state);
+}
+
dotraplinkage void __kprobes
do_general_protection(struct pt_regs *regs, long error_code)
{
--
1.7.1

2014-01-22 02:15:25

by Ren Qiaowei

[permalink] [raw]
Subject: [PATCH v2 4/4] x86, mpx: extend siginfo structure to include bound violation information

This patch adds new fields about bound violation into siginfo
structure. si_lower and si_upper are respectively lower bound
and upper bound when bound violation is caused.

These fields will be set in #BR exception handler by decoding
the user instruction and constructing the faulting pointer.
A userspace application can get violation address, lower bound
and upper bound for bound violation from this new siginfo structure.

Signed-off-by: Qiaowei Ren <[email protected]>
---
arch/x86/include/asm/mpx.h | 19 +++
arch/x86/kernel/mpx.c | 289 ++++++++++++++++++++++++++++++++++++
arch/x86/kernel/traps.c | 6 +
include/uapi/asm-generic/siginfo.h | 9 +-
kernel/signal.c | 4 +
5 files changed, 326 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
index 9652e9e..e099573 100644
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -3,6 +3,7 @@

#include <linux/types.h>
#include <asm/ptrace.h>
+#include <asm/insn.h>

#ifdef CONFIG_X86_64

@@ -30,6 +31,22 @@

#endif

+struct mpx_insn {
+ struct insn_field rex_prefix; /* REX prefix */
+ struct insn_field modrm;
+ struct insn_field sib;
+ struct insn_field displacement;
+
+ unsigned char addr_bytes; /* effective address size */
+ unsigned char limit;
+ unsigned char x86_64;
+
+ const unsigned char *kaddr; /* kernel address of insn to analyze */
+ const unsigned char *next_byte;
+};
+
+#define MAX_MPX_INSN_SIZE 15
+
typedef union {
struct {
unsigned long ignored:MPX_IGN_BITS;
@@ -40,5 +57,7 @@ typedef union {
} mpx_addr;

void do_mpx_bt_fault(struct xsave_struct *xsave_buf);
+void do_mpx_bounds(struct pt_regs *regs, siginfo_t *info,
+ struct xsave_struct *xsave_buf);

#endif /* _ASM_X86_MPX_H */
diff --git a/arch/x86/kernel/mpx.c b/arch/x86/kernel/mpx.c
index ffe5aee..3770991 100644
--- a/arch/x86/kernel/mpx.c
+++ b/arch/x86/kernel/mpx.c
@@ -91,6 +91,269 @@ int mpx_release(struct task_struct *tsk)
return 0;
}

+typedef enum {REG_TYPE_RM, REG_TYPE_INDEX, REG_TYPE_BASE} reg_type_t;
+static unsigned long get_reg(struct mpx_insn *insn, struct pt_regs *regs,
+ reg_type_t type)
+{
+ int regno = 0;
+ unsigned char modrm = (unsigned char)insn->modrm.value;
+ unsigned char sib = (unsigned char)insn->sib.value;
+
+ static const int regoff[] = {
+ offsetof(struct pt_regs, ax),
+ offsetof(struct pt_regs, cx),
+ offsetof(struct pt_regs, dx),
+ offsetof(struct pt_regs, bx),
+ offsetof(struct pt_regs, sp),
+ offsetof(struct pt_regs, bp),
+ offsetof(struct pt_regs, si),
+ offsetof(struct pt_regs, di),
+#ifdef CONFIG_X86_64
+ offsetof(struct pt_regs, r8),
+ offsetof(struct pt_regs, r9),
+ offsetof(struct pt_regs, r10),
+ offsetof(struct pt_regs, r11),
+ offsetof(struct pt_regs, r12),
+ offsetof(struct pt_regs, r13),
+ offsetof(struct pt_regs, r14),
+ offsetof(struct pt_regs, r15),
+#endif
+ };
+
+ switch (type) {
+ case REG_TYPE_RM:
+ regno = X86_MODRM_RM(modrm);
+ if (X86_REX_B(insn->rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ case REG_TYPE_INDEX:
+ regno = X86_SIB_INDEX(sib);
+ if (X86_REX_X(insn->rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ case REG_TYPE_BASE:
+ regno = X86_SIB_BASE(sib);
+ if (X86_REX_B(insn->rex_prefix.value) == 1)
+ regno += 8;
+ break;
+
+ default:
+ break;
+ }
+
+ return regs_get_register(regs, regoff[regno]);
+}
+
+/*
+ * return the address being referenced be instruction
+ * for rm=3 returning the content of the rm reg
+ * for rm!=3 calculates the address using SIB and Disp
+ */
+static unsigned long get_addr_ref(struct mpx_insn *insn, struct pt_regs *regs)
+{
+ unsigned long addr;
+ unsigned long base;
+ unsigned long indx;
+ unsigned char modrm = (unsigned char)insn->modrm.value;
+ unsigned char sib = (unsigned char)insn->sib.value;
+
+ if (X86_MODRM_MOD(modrm) == 3) {
+ addr = get_reg(insn, regs, REG_TYPE_RM);
+ } else {
+ if (insn->sib.nbytes) {
+ base = get_reg(insn, regs, REG_TYPE_BASE);
+ indx = get_reg(insn, regs, REG_TYPE_INDEX);
+ addr = base + indx * (1 << X86_SIB_SCALE(sib));
+ } else {
+ addr = get_reg(insn, regs, REG_TYPE_RM);
+ }
+ addr += insn->displacement.value;
+ }
+
+ return addr;
+}
+
+/* Verify next sizeof(t) bytes can be on the same instruction */
+#define validate_next(t, insn, n) \
+ ((insn)->next_byte + sizeof(t) + n - (insn)->kaddr <= (insn)->limit)
+
+#define __get_next(t, insn) \
+({ \
+ t r = *(t*)insn->next_byte; \
+ insn->next_byte += sizeof(t); \
+ r; \
+})
+
+#define __peek_next(t, insn) \
+({ \
+ t r = *(t*)insn->next_byte; \
+ r; \
+})
+
+#define get_next(t, insn) \
+({ \
+ if (unlikely(!validate_next(t, insn, 0))) \
+ goto err_out; \
+ __get_next(t, insn); \
+})
+
+#define peek_next(t, insn) \
+({ \
+ if (unlikely(!validate_next(t, insn, 0))) \
+ goto err_out; \
+ __peek_next(t, insn); \
+})
+
+static void mpx_insn_get_prefixes(struct mpx_insn *insn)
+{
+ unsigned char b;
+
+ /* Decode legacy prefix and REX prefix */
+ b = peek_next(unsigned char, insn);
+ while (b != 0x0f) {
+ /*
+ * look for a rex prefix
+ * a REX prefix cannot be followed by a legacy prefix.
+ */
+ if (insn->x86_64 && (b&0xf0)==0x40) {
+ insn->rex_prefix.value = b;
+ insn->rex_prefix.nbytes = 1;
+ insn->next_byte++;
+ break;
+ }
+
+ /* check the other legacy prefixes */
+ switch (b) {
+ case 0xf2:
+ case 0xf3:
+ case 0xf0:
+ case 0x64:
+ case 0x65:
+ case 0x2e:
+ case 0x3e:
+ case 0x26:
+ case 0x36:
+ case 0x66:
+ case 0x67:
+ insn->next_byte++;
+ break;
+ default: /* everything else is garbage */
+ goto err_out;
+ }
+ b = peek_next(unsigned char, insn);
+ }
+
+err_out:
+ return;
+}
+
+static void mpx_insn_get_modrm(struct mpx_insn *insn)
+{
+ insn->modrm.value = get_next(unsigned char, insn);
+ insn->modrm.nbytes = 1;
+
+err_out:
+ return;
+}
+
+static void mpx_insn_get_sib(struct mpx_insn *insn)
+{
+ unsigned char modrm = (unsigned char)insn->modrm.value;
+
+ if (X86_MODRM_MOD(modrm) != 3 && X86_MODRM_RM(modrm) == 4) {
+ insn->sib.value = get_next(unsigned char, insn);
+ insn->sib.nbytes = 1;
+ }
+
+err_out:
+ return;
+}
+
+static void mpx_insn_get_displacement(struct mpx_insn *insn)
+{
+ unsigned char mod, rm, base;
+
+ /*
+ * Interpreting the modrm byte:
+ * mod = 00 - no displacement fields (exceptions below)
+ * mod = 01 - 1-byte displacement field
+ * mod = 10 - displacement field is 4 bytes
+ * mod = 11 - no memory operand
+ *
+ * mod != 11, r/m = 100 - SIB byte exists
+ * mod = 00, SIB base = 101 - displacement field is 4 bytes
+ * mod = 00, r/m = 101 - rip-relative addressing, displacement
+ * field is 4 bytes
+ */
+ mod = X86_MODRM_MOD(insn->modrm.value);
+ rm = X86_MODRM_RM(insn->modrm.value);
+ base = X86_SIB_BASE(insn->sib.value);
+ if (mod == 3)
+ return;
+ if (mod == 1) {
+ insn->displacement.value = get_next(unsigned char, insn);
+ insn->displacement.nbytes = 1;
+ } else if ((mod == 0 && rm == 5) || mod == 2 ||
+ (mod == 0 && base == 5)) {
+ insn->displacement.value = get_next(int, insn);
+ insn->displacement.nbytes = 4;
+ }
+
+err_out:
+ return;
+}
+
+static void mpx_insn_init(struct mpx_insn *insn, struct pt_regs *regs)
+{
+ unsigned char buf[MAX_MPX_INSN_SIZE];
+ int bytes;
+
+ memset(insn, 0, sizeof(*insn));
+
+ bytes = copy_from_user(buf, (void __user *)regs->ip, MAX_MPX_INSN_SIZE);
+ insn->limit = MAX_MPX_INSN_SIZE - bytes;
+ insn->kaddr = buf;
+ insn->next_byte = buf;
+
+ /*
+ * In 64-bit Mode, all Intel MPX instructions use 64-bit
+ * operands for bounds and 64 bit addressing, i.e. REX.W &
+ * 67H have no effect on data or address size.
+ *
+ * In compatibility and legacy modes (including 16-bit code
+ * segments, real and virtual 8086 modes) all Intel MPX
+ * instructions use 32-bit operands for bounds and 32 bit
+ * addressing.
+ */
+#ifdef CONFIG_X86_64
+ insn->x86_64 = 1;
+ insn->addr_bytes = 8;
+#else
+ insn->x86_64 = 0;
+ insn->addr_bytes = 4;
+#endif
+}
+
+static unsigned long mpx_insn_decode(struct mpx_insn *insn, struct pt_regs *regs)
+{
+ mpx_insn_init(insn, regs);
+
+ /*
+ * In this case, we only need decode bndcl/bndcn/bndcu,
+ * so we can use private diassembly interfaces to get
+ * prefixes, modrm, sib, displacement, etc..
+ */
+ mpx_insn_get_prefixes(insn);
+ insn->next_byte += 2; /* ignore opcode */
+ mpx_insn_get_modrm(insn);
+ mpx_insn_get_sib(insn);
+ mpx_insn_get_displacement(insn);
+
+ return get_addr_ref(insn, regs);
+}
+
static bool allocate_bt(unsigned long bd_entry)
{
unsigned long bt_size = 1UL << (MPX_L2_BITS+MPX_L2_SHIFT);
@@ -126,3 +389,29 @@ void do_mpx_bt_fault(struct xsave_struct *xsave_buf)
if ((bd_entry >= bd_base) && (bd_entry < bd_base + bd_size))
allocate_bt(bd_entry);
}
+
+void do_mpx_bounds(struct pt_regs *regs, siginfo_t *info,
+ struct xsave_struct *xsave_buf)
+{
+ struct mpx_insn insn;
+ uint8_t bndregno;
+ unsigned long addr_vio;
+
+ addr_vio = mpx_insn_decode(&insn, regs);
+
+ bndregno = X86_MODRM_REG(insn.modrm.value);
+ if (bndregno > 3)
+ return;
+
+ info->si_lower =
+ (void __user *)(xsave_buf->bndregs.bndregs[2*bndregno]);
+ info->si_upper =
+ (void __user *)(~xsave_buf->bndregs.bndregs[2*bndregno+1]);
+ info->si_addr_lsb = 0;
+ info->si_signo = SIGSEGV;
+ info->si_errno = 0;
+ info->si_code = SEGV_BNDERR;
+ info->si_addr = (void __user *)addr_vio;
+ pr_debug("bound violation addr 0x%lx, lower 0x%lx, upper 0x%lx\n",
+ addr_vio, info->si_lower, info->si_upper);
+}
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 78e9c16..8bcb63b 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -273,6 +273,7 @@ dotraplinkage void do_bounds(struct pt_regs *regs, long error_code)
unsigned long status;
struct xsave_struct *xsave_buf;
struct task_struct *tsk = current;
+ siginfo_t info;

prev_state = exception_enter();
if (notify_die(DIE_TRAP, "bounds", regs, error_code,
@@ -308,6 +309,11 @@ dotraplinkage void do_bounds(struct pt_regs *regs, long error_code)
break;

case 1: /* Bound violation. */
+ do_mpx_bounds(regs, &info, xsave_buf);
+ do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs,
+ error_code, &info);
+ break;
+
case 0: /* No MPX exception. */
do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs, error_code, NULL);
break;
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index ba5be7f..1e35520 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -91,6 +91,10 @@ typedef struct siginfo {
int _trapno; /* TRAP # which caused the signal */
#endif
short _addr_lsb; /* LSB of the reported address */
+ struct {
+ void __user *_lower;
+ void __user *_upper;
+ } _addr_bnd;
} _sigfault;

/* SIGPOLL */
@@ -131,6 +135,8 @@ typedef struct siginfo {
#define si_trapno _sifields._sigfault._trapno
#endif
#define si_addr_lsb _sifields._sigfault._addr_lsb
+#define si_lower _sifields._sigfault._addr_bnd._lower
+#define si_upper _sifields._sigfault._addr_bnd._upper
#define si_band _sifields._sigpoll._band
#define si_fd _sifields._sigpoll._fd
#ifdef __ARCH_SIGSYS
@@ -199,7 +205,8 @@ typedef struct siginfo {
*/
#define SEGV_MAPERR (__SI_FAULT|1) /* address not mapped to object */
#define SEGV_ACCERR (__SI_FAULT|2) /* invalid permissions for mapped object */
-#define NSIGSEGV 2
+#define SEGV_BNDERR (__SI_FAULT|3) /* failed address bound checks */
+#define NSIGSEGV 3

/*
* SIGBUS si_codes
diff --git a/kernel/signal.c b/kernel/signal.c
index ded28b9..3cd6eaf 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2771,6 +2771,10 @@ int copy_siginfo_to_user(siginfo_t __user *to, siginfo_t *from)
if (from->si_code == BUS_MCEERR_AR || from->si_code == BUS_MCEERR_AO)
err |= __put_user(from->si_addr_lsb, &to->si_addr_lsb);
#endif
+#ifdef SEGV_BNDERR
+ err |= __put_user(from->si_lower, &to->si_lower);
+ err |= __put_user(from->si_upper, &to->si_upper);
+#endif
break;
case __SI_CHLD:
err |= __put_user(from->si_pid, &to->si_pid);
--
1.7.1

2014-01-22 08:00:48

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Intel MPX support

On Wed, 22 Jan 2014, Qiaowei Ren wrote:

> Changes since v1:
> * check to see if #BR occurred in userspace or kernel space.
> * use generic structure and macro as much as possible when
> decode mpx instructions.
>
> Qiaowei Ren (4):
> x86, mpx: add documentation on Intel MPX
> x86, mpx: hook #BR exception handler to allocate bound tables
> x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
> x86, mpx: extend siginfo structure to include bound violation
> information
>
> Documentation/x86/intel_mpx.txt | 76 +++++++
> arch/x86/Kconfig | 4 +
> arch/x86/include/asm/mpx.h | 63 ++++++
> arch/x86/include/asm/processor.h | 16 ++
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/mpx.c | 417 ++++++++++++++++++++++++++++++++++++
> arch/x86/kernel/traps.c | 61 +++++-
> include/uapi/asm-generic/siginfo.h | 9 +-
> include/uapi/linux/prctl.h | 6 +
> kernel/signal.c | 4 +
> kernel/sys.c | 12 +
> 11 files changed, 667 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/x86/intel_mpx.txt
> create mode 100644 arch/x86/include/asm/mpx.h
> create mode 100644 arch/x86/kernel/mpx.c
>

There's compiler warnings spread amongst the various patches with x86_64
defconfig:

arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’:
arch/x86/kernel/mpx.c:415:2: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘void *’ [-Wformat]
arch/x86/kernel/mpx.c:415:2: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘void *’ [-Wformat]
arch/x86/kernel/mpx.c: In function ‘do_mpx_bt_fault’:
arch/x86/kernel/mpx.c:373:5: warning: ‘old_val’ may be used uninitialized in this function [-Wuninitialized]
arch/x86/kernel/mpx.c:360:25: note: ‘old_val’ was declared here

and I had to resolve the second patch manually because of "x86/traps:
Clean up error exception handler definitions" in the x86 tree.

With 32-bit, we get casting warnings:

arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’:
arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

and I'm pretty sure you want this to be available for such a config.

There's also whitespace in the fourth patch.

2014-01-22 08:51:22

by Ren Qiaowei

[permalink] [raw]
Subject: RE: [PATCH v2 0/4] Intel MPX support

> -----Original Message-----
> From: David Rientjes [mailto:[email protected]]
> Sent: Wednesday, January 22, 2014 4:01 PM
> To: Ren, Qiaowei
> Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; [email protected];
> [email protected]
> Subject: Re: [PATCH v2 0/4] Intel MPX support
>
> On Wed, 22 Jan 2014, Qiaowei Ren wrote:
>
> > Changes since v1:
> > * check to see if #BR occurred in userspace or kernel space.
> > * use generic structure and macro as much as possible when
> > decode mpx instructions.
> >
> > Qiaowei Ren (4):
> > x86, mpx: add documentation on Intel MPX
> > x86, mpx: hook #BR exception handler to allocate bound tables
> > x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
> > x86, mpx: extend siginfo structure to include bound violation
> > information
> >
> > Documentation/x86/intel_mpx.txt | 76 +++++++
> > arch/x86/Kconfig | 4 +
> > arch/x86/include/asm/mpx.h | 63 ++++++
> > arch/x86/include/asm/processor.h | 16 ++
> > arch/x86/kernel/Makefile | 1 +
> > arch/x86/kernel/mpx.c | 417
> ++++++++++++++++++++++++++++++++++++
> > arch/x86/kernel/traps.c | 61 +++++-
> > include/uapi/asm-generic/siginfo.h | 9 +-
> > include/uapi/linux/prctl.h | 6 +
> > kernel/signal.c | 4 +
> > kernel/sys.c | 12 +
> > 11 files changed, 667 insertions(+), 2 deletions(-) create mode
> > 100644 Documentation/x86/intel_mpx.txt create mode 100644
> > arch/x86/include/asm/mpx.h create mode 100644 arch/x86/kernel/mpx.c
> >
>
> There's compiler warnings spread amongst the various patches with x86_64
> defconfig:
>
> arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’:
> arch/x86/kernel/mpx.c:415:2: warning: format ‘%lx’ expects argument of
> type ‘long unsigned int’, but argument 3 has type ‘void *’ [-Wformat]
> arch/x86/kernel/mpx.c:415:2: warning: format ‘%lx’ expects argument of
> type ‘long unsigned int’, but argument 4 has type ‘void *’ [-Wformat]
> arch/x86/kernel/mpx.c: In function ‘do_mpx_bt_fault’:
> arch/x86/kernel/mpx.c:373:5: warning: ‘old_val’ may be used uninitialized in
> this function [-Wuninitialized]
> arch/x86/kernel/mpx.c:360:25: note: ‘old_val’ was declared here
>
I will fix these warnings.
old_val will be set by user_atomic_cmpxchg_inatomic().

> and I had to resolve the second patch manually because of "x86/traps:
> Clean up error exception handler definitions" in the x86 tree.
>
There is a conflict with latest kernel. I just fixed it and built successfully.

> With 32-bit, we get casting warnings:
>
> arch/x86/kernel/mpx.c: In function ‘do_mpx_bounds’:
> arch/x86/kernel/mpx.c:407:3: warning: cast to pointer from integer of different
> size [-Wint-to-pointer-cast]
> arch/x86/kernel/mpx.c:409:3: warning: cast to pointer from integer of different
> size [-Wint-to-pointer-cast]
>
> and I'm pretty sure you want this to be available for such a config.
>
> There's also whitespace in the fourth patch.
Thanks much for your feedback.

Qiaowei

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-01-22 11:53:27

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Intel MPX support


* Qiaowei Ren <[email protected]> wrote:

> Changes since v1:
> * check to see if #BR occurred in userspace or kernel space.
> * use generic structure and macro as much as possible when
> decode mpx instructions.
>
> Qiaowei Ren (4):
> x86, mpx: add documentation on Intel MPX
> x86, mpx: hook #BR exception handler to allocate bound tables
> x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
> x86, mpx: extend siginfo structure to include bound violation
> information
>
> Documentation/x86/intel_mpx.txt | 76 +++++++
> arch/x86/Kconfig | 4 +
> arch/x86/include/asm/mpx.h | 63 ++++++
> arch/x86/include/asm/processor.h | 16 ++
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/mpx.c | 417 ++++++++++++++++++++++++++++++++++++
> arch/x86/kernel/traps.c | 61 +++++-
> include/uapi/asm-generic/siginfo.h | 9 +-
> include/uapi/linux/prctl.h | 6 +
> kernel/signal.c | 4 +
> kernel/sys.c | 12 +
> 11 files changed, 667 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/x86/intel_mpx.txt
> create mode 100644 arch/x86/include/asm/mpx.h
> create mode 100644 arch/x86/kernel/mpx.c

Such a patch submission is absolutely inadequate!

Please outline:

- a short summary of what the feature does

- a short description of what hardware supports it today or will
support it in the future

- a short description of whether the feature needs any
configuration from the user or it's entirely auto-enabled on
hardware that supports it.

- a cost/benefit description to unrelated code: is this slowing down
anything else?

- how does user-space compiler support stand, what's the expected
status there, etc.

Only a small fraction of that information can be found in
Documentation/x86/intel_mpx.txt. in

I'm absolutely sick of these semi-anonymous patch submissions from
Intel, so I'm NAK-ing it until it's communicated properly.

Thanks,

Ingo

2014-01-22 12:01:24

by Ren Qiaowei

[permalink] [raw]
Subject: RE: [PATCH v2 0/4] Intel MPX support



> -----Original Message-----
> From: Ingo Molnar [mailto:[email protected]] On Behalf Of Ingo
> Molnar
> Sent: Wednesday, January 22, 2014 7:53 PM
> To: Ren, Qiaowei
> Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; [email protected];
> [email protected]; Peter Zijlstra
> Subject: Re: [PATCH v2 0/4] Intel MPX support
>
>
> * Qiaowei Ren <[email protected]> wrote:
>
> > Changes since v1:
> > * check to see if #BR occurred in userspace or kernel space.
> > * use generic structure and macro as much as possible when
> > decode mpx instructions.
> >
> > Qiaowei Ren (4):
> > x86, mpx: add documentation on Intel MPX
> > x86, mpx: hook #BR exception handler to allocate bound tables
> > x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
> > x86, mpx: extend siginfo structure to include bound violation
> > information
> >
> > Documentation/x86/intel_mpx.txt | 76 +++++++
> > arch/x86/Kconfig | 4 +
> > arch/x86/include/asm/mpx.h | 63 ++++++
> > arch/x86/include/asm/processor.h | 16 ++
> > arch/x86/kernel/Makefile | 1 +
> > arch/x86/kernel/mpx.c | 417
> ++++++++++++++++++++++++++++++++++++
> > arch/x86/kernel/traps.c | 61 +++++-
> > include/uapi/asm-generic/siginfo.h | 9 +-
> > include/uapi/linux/prctl.h | 6 +
> > kernel/signal.c | 4 +
> > kernel/sys.c | 12 +
> > 11 files changed, 667 insertions(+), 2 deletions(-) create mode
> > 100644 Documentation/x86/intel_mpx.txt create mode 100644
> > arch/x86/include/asm/mpx.h create mode 100644 arch/x86/kernel/mpx.c
>
> Such a patch submission is absolutely inadequate!
>
> Please outline:
>
> - a short summary of what the feature does
>
> - a short description of what hardware supports it today or will
> support it in the future
>
> - a short description of whether the feature needs any
> configuration from the user or it's entirely auto-enabled on
> hardware that supports it.
>
> - a cost/benefit description to unrelated code: is this slowing down
> anything else?
>
> - how does user-space compiler support stand, what's the expected
> status there, etc.
>
> Only a small fraction of that information can be found in
> Documentation/x86/intel_mpx.txt. in
>
> I'm absolutely sick of these semi-anonymous patch submissions from Intel, so
> I'm NAK-ing it until it's communicated properly.
>
Ok. I will add related content into this documentation.

Thanks,
Qiaowei

2014-01-22 12:31:08

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Intel MPX support


* Ren, Qiaowei <[email protected]> wrote:

>
>
> > -----Original Message-----
> > From: Ingo Molnar [mailto:[email protected]] On Behalf Of Ingo
> > Molnar
> > Sent: Wednesday, January 22, 2014 7:53 PM
> > To: Ren, Qiaowei
> > Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; [email protected];
> > [email protected]; Peter Zijlstra
> > Subject: Re: [PATCH v2 0/4] Intel MPX support
> >
> >
> > * Qiaowei Ren <[email protected]> wrote:
> >
> > > Changes since v1:
> > > * check to see if #BR occurred in userspace or kernel space.
> > > * use generic structure and macro as much as possible when
> > > decode mpx instructions.
> > >
> > > Qiaowei Ren (4):
> > > x86, mpx: add documentation on Intel MPX
> > > x86, mpx: hook #BR exception handler to allocate bound tables
> > > x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
> > > x86, mpx: extend siginfo structure to include bound violation
> > > information
> > >
> > > Documentation/x86/intel_mpx.txt | 76 +++++++
> > > arch/x86/Kconfig | 4 +
> > > arch/x86/include/asm/mpx.h | 63 ++++++
> > > arch/x86/include/asm/processor.h | 16 ++
> > > arch/x86/kernel/Makefile | 1 +
> > > arch/x86/kernel/mpx.c | 417
> > ++++++++++++++++++++++++++++++++++++
> > > arch/x86/kernel/traps.c | 61 +++++-
> > > include/uapi/asm-generic/siginfo.h | 9 +-
> > > include/uapi/linux/prctl.h | 6 +
> > > kernel/signal.c | 4 +
> > > kernel/sys.c | 12 +
> > > 11 files changed, 667 insertions(+), 2 deletions(-) create mode
> > > 100644 Documentation/x86/intel_mpx.txt create mode 100644
> > > arch/x86/include/asm/mpx.h create mode 100644 arch/x86/kernel/mpx.c
> >
> > Such a patch submission is absolutely inadequate!
> >
> > Please outline:
> >
> > - a short summary of what the feature does
> >
> > - a short description of what hardware supports it today or will
> > support it in the future
> >
> > - a short description of whether the feature needs any
> > configuration from the user or it's entirely auto-enabled on
> > hardware that supports it.
> >
> > - a cost/benefit description to unrelated code: is this slowing down
> > anything else?
> >
> > - how does user-space compiler support stand, what's the expected
> > status there, etc.
> >
> > Only a small fraction of that information can be found in
> > Documentation/x86/intel_mpx.txt. in
> >
> > I'm absolutely sick of these semi-anonymous patch submissions from Intel, so
> > I'm NAK-ing it until it's communicated properly.
> >
> Ok. I will add related content into this documentation.

More importantly, put it into the 0/X mail! That's how people can
review such a patch set effectively.

Thanks,

Ingo

2014-01-23 01:48:15

by Ren Qiaowei

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Intel MPX support

On 01/22/2014 08:30 PM, Ingo Molnar wrote:
>
> * Ren, Qiaowei <[email protected]> wrote:
>
>>
>>
>>> -----Original Message-----
>>> From: Ingo Molnar [mailto:[email protected]] On Behalf Of Ingo
>>> Molnar
>>> Sent: Wednesday, January 22, 2014 7:53 PM
>>> To: Ren, Qiaowei
>>> Cc: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; [email protected];
>>> [email protected]; Peter Zijlstra
>>> Subject: Re: [PATCH v2 0/4] Intel MPX support
>>>
>>>
>>> * Qiaowei Ren <[email protected]> wrote:
>>>
>>>> Changes since v1:
>>>> * check to see if #BR occurred in userspace or kernel space.
>>>> * use generic structure and macro as much as possible when
>>>> decode mpx instructions.
>>>>
>>>> Qiaowei Ren (4):
>>>> x86, mpx: add documentation on Intel MPX
>>>> x86, mpx: hook #BR exception handler to allocate bound tables
>>>> x86, mpx: add prctl commands PR_MPX_INIT, PR_MPX_RELEASE
>>>> x86, mpx: extend siginfo structure to include bound violation
>>>> information
>>>>
>>>> Documentation/x86/intel_mpx.txt | 76 +++++++
>>>> arch/x86/Kconfig | 4 +
>>>> arch/x86/include/asm/mpx.h | 63 ++++++
>>>> arch/x86/include/asm/processor.h | 16 ++
>>>> arch/x86/kernel/Makefile | 1 +
>>>> arch/x86/kernel/mpx.c | 417
>>> ++++++++++++++++++++++++++++++++++++
>>>> arch/x86/kernel/traps.c | 61 +++++-
>>>> include/uapi/asm-generic/siginfo.h | 9 +-
>>>> include/uapi/linux/prctl.h | 6 +
>>>> kernel/signal.c | 4 +
>>>> kernel/sys.c | 12 +
>>>> 11 files changed, 667 insertions(+), 2 deletions(-) create mode
>>>> 100644 Documentation/x86/intel_mpx.txt create mode 100644
>>>> arch/x86/include/asm/mpx.h create mode 100644 arch/x86/kernel/mpx.c
>>>
>>> Such a patch submission is absolutely inadequate!
>>>
>>> Please outline:
>>>
>>> - a short summary of what the feature does
>>>
>>> - a short description of what hardware supports it today or will
>>> support it in the future
>>>
>>> - a short description of whether the feature needs any
>>> configuration from the user or it's entirely auto-enabled on
>>> hardware that supports it.
>>>
>>> - a cost/benefit description to unrelated code: is this slowing down
>>> anything else?
>>>
>>> - how does user-space compiler support stand, what's the expected
>>> status there, etc.
>>>
>>> Only a small fraction of that information can be found in
>>> Documentation/x86/intel_mpx.txt. in
>>>
>>> I'm absolutely sick of these semi-anonymous patch submissions from Intel, so
>>> I'm NAK-ing it until it's communicated properly.
>>>
>> Ok. I will add related content into this documentation.
>
> More importantly, put it into the 0/X mail! That's how people can
> review such a patch set effectively.
>
Ok. Thanks for your feedback. I will do it.

Thanks,
Qiaowei