From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757693AbbDVS1p (ORCPT ); Wed, 22 Apr 2015 14:27:45 -0400 Received: from mga03.intel.com ([134.134.136.65]:52313 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757665AbbDVS1m (ORCPT ); Wed, 22 Apr 2015 14:27:42 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,625,1422950400"; d="scan'208";a="717617153" Subject: [PATCH 02/17] x86, mpx: use new tsk_get_xsave_addr() To: linux-kernel@vger.kernel.org Cc: x86@kernel.org, tglx@linutronix.de, Dave Hansen , dave.hansen@linux.intel.com, oleg@redhat.com, bp@alien8.de, riel@redhat.com, sbsiddha@gmail.com, luto@amacapital.net, mingo@redhat.com, hpa@zytor.com, fenghua.yu@intel.com From: Dave Hansen Date: Wed, 22 Apr 2015 11:27:32 -0700 References: <20150422182729.0512E57D@viggo.jf.intel.com> In-Reply-To: <20150422182729.0512E57D@viggo.jf.intel.com> Message-Id: <20150422182732.91684156@viggo.jf.intel.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Dave Hansen 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 Cc: Oleg Nesterov Cc: bp@alien8.de Cc: Rik van Riel Cc: Suresh Siddha Cc: Andy Lutomirski Cc: Thomas Gleixner Cc: Ingo Molnar Cc: "H. Peter Anvin" Cc: Fenghua Yu Cc: the arch/x86 maintainers Cc: linux-kernel --- b/arch/x86/include/asm/mpx.h | 8 ++++---- b/arch/x86/kernel/traps.c | 10 ++++------ b/arch/x86/mm/mpx.c | 23 +++++++++++------------ 3 files changed, 19 insertions(+), 22 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-04-22 11:16:18.188818959 -0700 +++ b/arch/x86/include/asm/mpx.h 2015-04-22 11:16:18.195819274 -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-04-22 11:16:18.190819049 -0700 +++ b/arch/x86/kernel/traps.c 2015-04-22 11:16:18.195819274 -0700 @@ -61,6 +61,7 @@ #include #include #include +#include #ifdef CONFIG_X86_64 #include @@ -373,7 +374,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; @@ -397,9 +397,7 @@ dotraplinkage void do_bounds(struct pt_r * It is not directly accessible, though, so we need to * do an xsave and then pull it out of the xsave buffer. */ - 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; @@ -410,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-04-22 11:16:18.192819139 -0700 +++ b/arch/x86/mm/mpx.c 2015-04-22 11:16:18.196819320 -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 _