lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20150508185949.3250100D@viggo.jf.intel.com>
Date:	Fri, 08 May 2015 11:59:49 -0700
From:	Dave Hansen <dave@...1.net>
To:	linux-kernel@...r.kernel.org
Cc:	x86@...nel.org, tglx@...utronix.de, Dave Hansen <dave@...1.net>,
	dave.hansen@...ux.intel.com, oleg@...hat.com, bp@...en8.de,
	riel@...hat.com, sbsiddha@...il.com, luto@...capital.net,
	mingo@...hat.com, hpa@...or.com, fenghua.yu@...el.com
Subject: [PATCH 03/19] x86, mpx: use new tsk_get_xsave_addr()


From: Dave Hansen <dave.hansen@...ux.intel.com>

The MPX registers (bndcsr/bndcfgu/bndstatus) are not directly
accessible via normal instructions.  They essentially act as
if they were floating point registers and are saved/restored
along with those registers.

There are two main paths in the MPX code where we care about
the contents of these registers:
	1. #BR (bounds) faults
	2. the prctl() code where we are setting MPX up

Both of those paths _might_ be called without the FPU having
been used.  That means that 'tsk->thread.fpu.state' might
never be allocated.

Also, fpu_save_init() is not preempt-safe.  It was a bug to
call it without disabling preemption.  The new
tsk_get_xsave_addr() calls unlazy_fpu() instead and properly
disables preemption.

Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com>
Cc: Oleg Nesterov <oleg@...hat.com>
Cc: bp@...en8.de
Cc: Rik van Riel <riel@...hat.com>
Cc: Suresh Siddha <sbsiddha@...il.com>
Cc: Andy Lutomirski <luto@...capital.net>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: "H. Peter Anvin" <hpa@...or.com>
Cc: Fenghua Yu <fenghua.yu@...el.com>
Cc: the arch/x86 maintainers <x86@...nel.org>
Cc: linux-kernel <linux-kernel@...r.kernel.org>
---

 b/arch/x86/include/asm/mpx.h |    8 ++++----
 b/arch/x86/kernel/traps.c    |   15 +++++++--------
 b/arch/x86/mm/mpx.c          |   23 +++++++++++------------
 3 files changed, 22 insertions(+), 24 deletions(-)

diff -puN arch/x86/include/asm/mpx.h~use-new-tsk_get_xsave_addr arch/x86/include/asm/mpx.h
--- a/arch/x86/include/asm/mpx.h~use-new-tsk_get_xsave_addr	2015-05-08 11:46:11.385599446 -0700
+++ b/arch/x86/include/asm/mpx.h	2015-05-08 11:46:11.392599762 -0700
@@ -60,8 +60,8 @@
 
 #ifdef CONFIG_X86_INTEL_MPX
 siginfo_t *mpx_generate_siginfo(struct pt_regs *regs,
-				struct xsave_struct *xsave_buf);
-int mpx_handle_bd_fault(struct xsave_struct *xsave_buf);
+				struct task_struct *tsk);
+int mpx_handle_bd_fault(struct task_struct *tsk);
 static inline int kernel_managing_mpx_tables(struct mm_struct *mm)
 {
 	return (mm->bd_addr != MPX_INVALID_BOUNDS_DIR);
@@ -78,11 +78,11 @@ void mpx_notify_unmap(struct mm_struct *
 		      unsigned long start, unsigned long end);
 #else
 static inline siginfo_t *mpx_generate_siginfo(struct pt_regs *regs,
-					      struct xsave_struct *xsave_buf)
+					      struct task_struct *tsk)
 {
 	return NULL;
 }
-static inline int mpx_handle_bd_fault(struct xsave_struct *xsave_buf)
+static inline int mpx_handle_bd_fault(struct task_struct *tsk)
 {
 	return -EINVAL;
 }
diff -puN arch/x86/kernel/traps.c~use-new-tsk_get_xsave_addr arch/x86/kernel/traps.c
--- a/arch/x86/kernel/traps.c~use-new-tsk_get_xsave_addr	2015-05-08 11:46:11.387599536 -0700
+++ b/arch/x86/kernel/traps.c	2015-05-08 11:46:11.392599762 -0700
@@ -61,6 +61,7 @@
 #include <asm/mach_traps.h>
 #include <asm/alternative.h>
 #include <asm/mpx.h>
+#include <asm/xsave.h>
 
 #ifdef CONFIG_X86_64
 #include <asm/x86_init.h>
@@ -372,7 +373,6 @@ dotraplinkage void do_double_fault(struc
 dotraplinkage void do_bounds(struct pt_regs *regs, long error_code)
 {
 	struct task_struct *tsk = current;
-	struct xsave_struct *xsave_buf;
 	enum ctx_state prev_state;
 	struct bndcsr *bndcsr;
 	siginfo_t *info;
@@ -393,12 +393,11 @@ dotraplinkage void do_bounds(struct pt_r
 
 	/*
 	 * We need to look at BNDSTATUS to resolve this exception.
-	 * It is not directly accessible, though, so we need to
-	 * do an xsave and then pull it out of the xsave buffer.
+	 * A NULL here might mean that it is in its 'init state',
+	 * which is all zeros which indicates MPX was not
+	 * responsible for the exception.
 	 */
-	fpu_save_init(&tsk->thread.fpu);
-	xsave_buf = &(tsk->thread.fpu.state->xsave);
-	bndcsr = get_xsave_addr(xsave_buf, XSTATE_BNDCSR);
+	bndcsr = get_xsave_field(XSTATE_BNDCSR);
 	if (!bndcsr)
 		goto exit_trap;
 
@@ -409,11 +408,11 @@ dotraplinkage void do_bounds(struct pt_r
 	 */
 	switch (bndcsr->bndstatus & MPX_BNDSTA_ERROR_CODE) {
 	case 2:	/* Bound directory has invalid entry. */
-		if (mpx_handle_bd_fault(xsave_buf))
+		if (mpx_handle_bd_fault(tsk))
 			goto exit_trap;
 		break; /* Success, it was handled */
 	case 1: /* Bound violation. */
-		info = mpx_generate_siginfo(regs, xsave_buf);
+		info = mpx_generate_siginfo(regs, tsk);
 		if (IS_ERR(info)) {
 			/*
 			 * We failed to decode the MPX instruction.  Act as if
diff -puN arch/x86/mm/mpx.c~use-new-tsk_get_xsave_addr arch/x86/mm/mpx.c
--- a/arch/x86/mm/mpx.c~use-new-tsk_get_xsave_addr	2015-05-08 11:46:11.388599581 -0700
+++ b/arch/x86/mm/mpx.c	2015-05-08 11:46:11.393599807 -0700
@@ -273,7 +273,7 @@ bad_opcode:
  * The caller is expected to kfree() the returned siginfo_t.
  */
 siginfo_t *mpx_generate_siginfo(struct pt_regs *regs,
-				struct xsave_struct *xsave_buf)
+				struct task_struct *tsk)
 {
 	struct bndreg *bndregs, *bndreg;
 	siginfo_t *info = NULL;
@@ -295,8 +295,8 @@ siginfo_t *mpx_generate_siginfo(struct p
 		err = -EINVAL;
 		goto err_out;
 	}
-	/* get the bndregs _area_ of the xsave structure */
-	bndregs = get_xsave_addr(xsave_buf, XSTATE_BNDREGS);
+	/* get bndregs field from urrent task's xsave area */
+	bndregs = get_xsave_field(XSTATE_BNDREGS);
 	if (!bndregs) {
 		err = -EINVAL;
 		goto err_out;
@@ -358,8 +358,7 @@ static __user void *task_get_bounds_dir(
 	 * The bounds directory pointer is stored in a register
 	 * only accessible if we first do an xsave.
 	 */
-	fpu_save_init(&tsk->thread.fpu);
-	bndcsr = get_xsave_addr(&tsk->thread.fpu.state->xsave, XSTATE_BNDCSR);
+	bndcsr = get_xsave_field(XSTATE_BNDCSR);
 	if (!bndcsr)
 		return MPX_INVALID_BOUNDS_DIR;
 
@@ -390,9 +389,9 @@ int mpx_enable_management(struct task_st
 	 * directory into XSAVE/XRSTOR Save Area and enable MPX through
 	 * XRSTOR instruction.
 	 *
-	 * fpu_xsave() is expected to be very expensive. Storing the bounds
-	 * directory here means that we do not have to do xsave in the unmap
-	 * path; we can just use mm->bd_addr instead.
+	 * xsaves are expected to be very expensive.  Storing the bounds
+	 * directory here means that we do not have to do xsave in the
+	 * unmap path; we can just use mm->bd_addr instead.
 	 */
 	bd_base = task_get_bounds_dir(tsk);
 	down_write(&mm->mmap_sem);
@@ -498,12 +497,12 @@ out_unmap:
  * bound table is 16KB. With 64-bit mode, the size of BD is 2GB,
  * and the size of each bound table is 4MB.
  */
-static int do_mpx_bt_fault(struct xsave_struct *xsave_buf)
+static int do_mpx_bt_fault(struct task_struct *tsk)
 {
 	unsigned long bd_entry, bd_base;
 	struct bndcsr *bndcsr;
 
-	bndcsr = get_xsave_addr(xsave_buf, XSTATE_BNDCSR);
+	bndcsr = get_xsave_field(XSTATE_BNDCSR);
 	if (!bndcsr)
 		return -EINVAL;
 	/*
@@ -526,7 +525,7 @@ static int do_mpx_bt_fault(struct xsave_
 	return allocate_bt((long __user *)bd_entry);
 }
 
-int mpx_handle_bd_fault(struct xsave_struct *xsave_buf)
+int mpx_handle_bd_fault(struct task_struct *tsk)
 {
 	/*
 	 * Userspace never asked us to manage the bounds tables,
@@ -535,7 +534,7 @@ int mpx_handle_bd_fault(struct xsave_str
 	if (!kernel_managing_mpx_tables(current->mm))
 		return -EINVAL;
 
-	if (do_mpx_bt_fault(xsave_buf)) {
+	if (do_mpx_bt_fault(tsk)) {
 		force_sig(SIGSEGV, current);
 		/*
 		 * The force_sig() is essentially "handling" this
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ