| 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
| ||
|
Date: Wed, 22 Apr 2015 19:18:23 +0000
From: "Yu, Fenghua" <fenghua.yu@...el.com>
To: Dave Hansen <dave@...1.net>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "x86@...nel.org" <x86@...nel.org>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"oleg@...hat.com" <oleg@...hat.com>, "bp@...en8.de" <bp@...en8.de>,
"riel@...hat.com" <riel@...hat.com>,
"sbsiddha@...il.com" <sbsiddha@...il.com>,
"luto@...capital.net" <luto@...capital.net>,
"mingo@...hat.com" <mingo@...hat.com>,
"hpa@...or.com" <hpa@...or.com>
Subject: RE: [PATCH 02/17] x86, mpx: use new tsk_get_xsave_addr()
> From: Dave Hansen [mailto:dave@...1.net]
> Sent: Wednesday, April 22, 2015 11:28 AM
> To: linux-kernel@...r.kernel.org
> Cc: x86@...nel.org; tglx@...utronix.de; Dave Hansen;
> 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; Yu, Fenghua
> Subject: [PATCH 02/17] 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 | 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 <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>
> @@ -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)
> {
Do you need the parameter "tsk"? It's not used in this function.
Seems this function only needs to take one parameter "regs". Other parameters
are unnecessary.
> 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)
Do you need the parameter "tsk"? It's not used in this function.
Seems this function doesn't need to take any parameter.
> {
> 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