x86, mpx updates for 4.1 (take 4)
Subsystems (4)
cover x86, mpx updates for 4.1 (take 4)
Changes from take 3 / v18 (all minor):
* use DECLARE_EVENT_CLASS()/DEFINE_EVENT() for the ranged tracepoints to save 10 lines of code.
Changes from take 2 / v17 (all minor):
* fix a couple of whitespace borkages caught by checkpatch, and a spelling error or two.
* replace printk with pr_info() for boot disable
* change trace print format for address intervals
* fix up variable name in tsk_get_xsave_addr() comment
* remove tsk_get_xsave_field() GPL export
* fix up Qiaowei's From:
--
Hi x86 maintainers,
There are 4 basic things going on here:
1. Make FPU/xsave code preempt safe and work properly
2. Add trace points to make kernel and app debugging easier
3. Add a boot-time disable for mpx
4. Support 32-bit binaries to run on 64-bit kernels
I've hesitated sending this in the past few weeks as the FPU code had a lot of churn and affected our ability to test on current kernels. It seems to have settled down and appears more stable now.
As mentioned in the tsk_get_xsave_addr() patches, this must be applied after Oleg's fixes which currently ends with:
commit 7fc253e27 ("x86/fpu: Kill eager_fpu_init_bp()")This set is also available against 4.0-rc4 in git:
git://git.kernel.org/pub/scm/linux/kernel/git/daveh/x86-mpx.git mpx-v19
Dave Hansen (16):
x86, fpu: wrap get_xsave_addr() to make it safer x86, mpx: use new tsk_get_xsave_addr() x86, mpx: trace #BR exceptions
x86, mpx: trace entry to bounds exception paths x86, mpx: trace ranged MPX operations
x86, mpx: trace allocation of new bounds tables x86, mpx: boot-time disable
x86: make is_64bit_mm() widely available
x86: make __VIRTUAL_MASK safe to use on 32 bit x86, mpx: we do not allocate the bounds directory x86, mpx: remove redundant MPX_BNDCFG_ADDR_MASK x86, mpx: Add temporary variable to reduce masking x86, mpx: new directory entry to addr helper x86, mpx: do 32-bit-only cmpxchg for 32-bit apps x86, mpx: support 32-bit binaries on 64-bit kernel x86, mpx: allow mixed binaries again
Documentation/kernel-parameters.txt | 4 + arch/x86/include/asm/mmu_context.h | 13 ++ arch/x86/include/asm/mpx.h | 74 +++++----- arch/x86/include/asm/page_types.h | 8 ++ arch/x86/include/asm/trace/mpx.h | 121 ++++++++++++++++ arch/x86/include/asm/xsave.h | 1 + arch/x86/kernel/cpu/common.c | 16 +++ arch/x86/kernel/traps.c | 12 +- arch/x86/kernel/uprobes.c | 10 +- arch/x86/kernel/xsave.c | 31 +++++ arch/x86/mm/mpx.c | 270 +++++++++++++++++++++++++++++------- 11 files changed, 456 insertions(+), 104 deletions(-) create mode 100644 arch/x86/include/asm/trace/mpx.h
PATCH 1/16 x86, fpu: wrap get_xsave_addr() to make it safer
Changes from "v17":
* fix s/xstate/xsave_field/ in the function comment
* remove EXPORT_SYMBOL_GPL()
---
From: Dave Hansen <[email protected]>
The MPX code appears to be saving off the FPU in an unsafe
way. It does not disable preemption or ensure that the
FPU state has been allocated.
This patch introduces a new helper which will do both of those things internally.
Note that this requires a patch from Oleg in order to work properly. It is currently in tip/x86/fpu.
Quoted 5 lines
commit
f893959b0898bd876673adbeb6798bdf25c034d7Author: Oleg Nesterov <[email protected]> Date: Fri Mar 13 18:30:30 2015 +0100x86/fpu: Don't abuse
drop_init_fpu()inflush_thread()
---
b/arch/x86/include/asm/xsave.h | 1 + b/arch/x86/kernel/xsave.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+)
arch/x86/include/asm/xsave.h renamed from arch/x86/include/asm/xsave.h~tsk_get_xsave_addr +1 -0
@@ -252,6 +252,7 @@ static inline int xrestore_user(struct x }void *get_xsave_addr (struct xsave_struct *xsave,int xstate); +void *tsk_get_xsave_field (struct task_struct *tsk,int xstate_field);void setup_xstate_comp (void );#endif
arch/x86/kernel/xsave.c renamed from arch/x86/kernel/xsave.c~tsk_get_xsave_addr +31 -0
@@ -722,3 +722,34 @@ void *get_xsave_addr(struct xsave_structreturn (void *)xsave + xstate_comp_offsets[feature]; }EXPORT_SYMBOL_GPL (get_xsave_addr); + +/* +* This wraps up the common operations that need to occur when retrieving +* data from an xsave struct. It first ensures that the task was actually +* using the FPU and retrieves the data in to a buffer. It then calculates +* the offset of the requested field in the buffer. +* +* This function is safe to call whether the FPU is in use or not. +* +* Inputs: +* @tsk: the task from which we are fetching xsave state +* @xsave_field: state which is defined in xsave.h (e.g. XSTATE_FP, +* XSTATE_SSE, etc...) +* Output: +* address of the state in the xsave area. +*/ +void *tsk_get_xsave_field (struct task_struct *tsk,int xsave_field) +{ +union thread_xstate *xstate; + +if (!used_math ()) +return NULL ; +/* +* unlazy_fpu() is poorly named and will actually +* save the xstate off in to the memory buffer. +*/ +unlazy_fpu (tsk); + xstate = tsk->thread.fpu.state; + +return get_xsave_addr (&xstate->xsave, xsave_field); +}
_
Borislav Petkov
maintainerRe: [PATCH 01/16] x86, fpu: wrap get_xsave_addr() to make it safer
Quoted 11 lines · Dave Hansen
arch/x86/kernel/xsave.c renamed from arch/x86/kernel/xsave.c~tsk_get_xsave_addr +11
-0+/* +* This wraps up the common operations that need to occur when retrieving +* data from an xsave struct. It first ensures that the task was actually +* using the FPU and retrieves the data in to a buffer. It then calculates +* the offset of the requested field in the buffer. +* +* This function is safe to call whether the FPU is in use or not. +* +* Inputs: +* @tsk: the task from which we are fetching xsave state +* @xsave_field: state which is defined in xsave.h (e.g. XSTATE_FP,
arch/x86/kernel/xsave.c void *get_xsave_addr(struct xsave_struct · line 746
Quoted 10 lines
arch/x86/kernel/xsave.c renamed from arch/x86/kernel/xsave.c~tsk_get_xsave_addr +10
-0+* XSTATE_SSE, etc...) +* Output: +* address of the state in the xsave area. +*/ +void *tsk_get_xsave_field (struct task_struct *tsk,int xsave_field) +{ +union thread_xstate *xstate; + +if (!used_math ()) +return NULL ;
Shouldn't this be
if (!tsk_used_math(tsk))
?
Because used_math() is looking at current, maybe even in preemption-enabled paths - I'm eyeing task_get_bounds_dir() - and that current might get changed from under us and it might happen that current != tsk. Yes, no?
arch/x86/kernel/xsave.c void *get_xsave_addr(struct xsave_struct · line 754
Quoted 8 lines
arch/x86/kernel/xsave.c renamed from arch/x86/kernel/xsave.c~tsk_get_xsave_addr +8
-0+/* +* unlazy_fpu() is poorly named and will actually +* save the xstate off in to the memory buffer. +*/ +unlazy_fpu (tsk); + xstate = tsk->thread.fpu.state; + +return get_xsave_addr (&xstate->xsave, xsave_field);
And I understand this as "give me the xsave address of @tsk".
Right?
Thanks.
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Oleg Nesterov
maintainerRe: [PATCH 01/16] x86, fpu: wrap get_xsave_addr() to make it safer
Quoted 19 lines · Borislav Petkov
Quoted 8 lines · Dave Hansen
+ *
+ * Inputs:
+ * @tsk: the task from which we are fetching xsave state
+ * @xsave_field: state which is defined inxsave.h(e.g.XSTATE_FP,
+ *XSTATE_SSE, etc...)
+ * Output:
+ * address of the state in the xsave area.
+ */arch/x86/kernel/xsave.c void *get_xsave_addr(struct xsave_struct · line 746
Quoted 6 lines
arch/x86/kernel/xsave.c renamed from arch/x86/kernel/xsave.c~tsk_get_xsave_addr +6
-0+void *tsk_get_xsave_field (struct task_struct *tsk,int xsave_field) +{ +union thread_xstate *xstate; + +if (!used_math ()) +return NULL ;Shouldn't this be
Quoted 3 lines
if (!tsk_used_math(tsk))?
I agree, tsk_used_math(tsk) looks better, simpy because we have this argument.
But this "tsk" should be always current, otherwise this code is wrong anyway. Say, unlazy_fpu(tsk) can't work if tsk != current.
So perhaps the comment should be updated...
Quoted 4 lines
Because
used_math()is looking at current, maybe even in preemption-enabled paths - I'm eyeingtask_get_bounds_dir()- and that current might get changed from under us and it might happen that current != tsk. Yes, no?
Not sure I understand... "current" can't change from under us?
Even if this CPU switches to another thread which executes the same code, that thread will obviously see another "current", but its "tsk" variable will still match its "current".
Oleg.
Borislav Petkov
maintainerRe: [PATCH 01/16] x86, fpu: wrap get_xsave_addr() to make it safer
Quoted 4 lines · Oleg Nesterov
I agree,
tsk_used_math(tsk)looks better, simpy because we have this argument.But this "tsk" should be always current, otherwise this code is wrong
This is exactly what I'm asking: is that always the case?...
Quoted 10 lines
anyway. Say,
unlazy_fpu(tsk)can't work if tsk != current.So perhaps the comment should be updated...
Quoted 4 lines
Because
used_math()is looking at current, maybe even in preemption-enabled paths - I'm eyeingtask_get_bounds_dir()- and that current might get changed from under us and it might happen that current != tsk. Yes, no?Not sure I understand... "current" can't change from under us?
... I'm not sure all tsk_get_xsave_field() callers disable preemption. If not, then current can change from under us...
Quoted 3 lines
Even if this CPU switches to another thread which executes the same code, that thread will obviously see another "current", but its "tsk" variable will still match its "current".
Well, we want to see if @tsk used math, not necessarily if current used math, especially if it is another task, right?
I read tsk_get_xsave_field(@tsk, ) as give me the xsave field of @tsk but doing used_math() we're querying current and I'm not sure
tsk == current
in all the call sites of tsk_get_xsave_field().
Does that make more sense?
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Re: [PATCH 01/16] x86, fpu: wrap get_xsave_addr() to make it safer
Quoted 6 lines · Borislav Petkov
I read
tsk_get_xsave_field(@tsk, )as give me the xsave field of @tsk but doingused_math()we're querying current and I'm not suretsk == currentin all the call sites of
tsk_get_xsave_field().
There aren't that many call sites. :)
But, yeah, tsk==current for all of them at the moment.
Re: [PATCH 01/16] x86, fpu: wrap get_xsave_addr() to make it safer
Quoted 11 lines · Dave Hansen
Quoted 6 lines · Borislav Petkov
I read
tsk_get_xsave_field(@tsk, )as give me the xsave field of @tsk but doingused_math()we're querying current and I'm not suretsk == currentin all the call sites of
tsk_get_xsave_field().There aren't that many call sites. :)
But, yeah, tsk==current for all of them at the moment.
Is there a reason to have the task argument at all if thats only intended for current?
Thanks,
tglx
Borislav Petkov
maintainerRe: [PATCH 01/16] x86, fpu: wrap get_xsave_addr() to make it safer
Quoted 2 lines · Thomas Gleixner
Is there a reason to have the task argument at all if thats only intended for current?
Right, so I think we either use @tsk everywhere or disable preemption in the whole function if using current. But I think we want to use @tsk...
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Re: [PATCH 01/16] x86, fpu: wrap get_xsave_addr() to make it safer
Quoted 6 lines · Borislav Petkov
Quoted 2 lines · Thomas Gleixner
Is there a reason to have the task argument at all if thats only intended for current?
Right, so I think we either use @tsk everywhere or disable preemption in the whole function if using current. But I think we want to use @tsk...
Why should we disable preemption?
Thanks,
tglx
Re: [PATCH 01/16] x86, fpu: wrap get_xsave_addr() to make it safer
In reply to Thomas GleixnerQuoted 7 lines · Thomas Gleixner
Quoted 6 lines · Dave Hansen
Quoted 5 lines · Borislav Petkov
I read
tsk_get_xsave_field(@tsk, )as give me the xsave field of @tsk but doingused_math()we're querying current and I'm not suretsk == current
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
in all the call sites of
tsk_get_xsave_field().There aren't that many call sites. :)
But, yeah, tsk==current for all of them at the moment.
Is there a reason to have the task argument at all if thats only intended for current?
No, there is no good reason. I'll rework it and resend the set.
Oleg Nesterov
maintainerRe: [PATCH 01/16] x86, fpu: wrap get_xsave_addr() to make it safer
In reply to Borislav PetkovQuoted 8 lines · Borislav Petkov
Quoted 4 lines · Oleg Nesterov
I agree,
tsk_used_math(tsk)looks better, simpy because we have this argument.But this "tsk" should be always current, otherwise this code is wrong
This is exactly what I'm asking: is that always the case?...
I can't look at these patches now, but iirc - yes. The caller is either prctl() or exception. Dave will correct me.
Otherwise, once again, this code is simply buggy. So the comment should probably explain this.
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Because
used_math()is looking at current, maybe even in preemption-enabled paths - I'm eyeingtask_get_bounds_dir()- and that current might get changed from under us and it might happen that current != tsk. Yes, no?Not sure I understand... "current" can't change from under us?
... I'm not sure all
tsk_get_xsave_field()callers disable preemption. If not, then current can change from under us...
How? I am certainly missing you point... OK, please forge about FPU. Consider this code:
tsk = current; for (;;) BUG_ON(tsk != current);
it doesn't need to disable preemption. We do not care if CPU switches to another thread, even if this thread executes the same code. Because its tsk/current will differ, but "tsk == current" will be still true.
Could you please spell?
Quoted 6 lines
Quoted 3 lines
Even if this CPU switches to another thread which executes the same code, that thread will obviously see another "current", but its "tsk" variable will still match its "current".
Well, we want to see if @tsk used math, not necessarily if current used math, especially if it is another task, right?
See above... used_math() should be correct because we know that tsk==current, but I agree that tsk_used_math(tsk) looks better.
Quoted 6 lines
I read
tsk_get_xsave_field(@tsk, )as give me the xsave field of @tsk but doingused_math()we're querying current and I'm not suretsk == currentin all the call sites of
tsk_get_xsave_field().
Yes, the name/comment looks confusing a bit, as if you can use it when tsk != current...
Oleg.
Borislav Petkov
maintainerRe: [PATCH 01/16] x86, fpu: wrap get_xsave_addr() to make it safer
Quoted 7 lines · Oleg Nesterov
How? I am certainly missing you point... OK, please forge about FPU. Consider this code:
tsk = current; for (;;) BUG_ON(tsk != current);
Quoted 5 lines
it doesn't need to disable preemption. We do not care if CPU switches to another thread, even if this thread executes the same code. Because its tsk/current will differ, but "tsk == current" will be still true.
Could you please spell?
Bah, forget what I said. tglx schooled me on IRC. We always do get called with tsk == current. Sorry for the confusion...
Quoted 2 lines
See above...
used_math()should be correct because we know that tsk==current, but I agree thattsk_used_math(tsk)looks better.
Right, and it is clear what's going on, even to lusers like me.
Thanks for teaching me guys.
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--PATCH 2/16 x86, mpx: use new tsk_get_xsave_addr()
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
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.
---
b/arch/x86/include/asm/mpx.h | 8 ++++---- b/arch/x86/kernel/traps.c | 10 ++++------ b/arch/x86/mm/mpx.c | 21 ++++++++++----------- 3 files changed, 18 insertions(+), 21 deletions(-)
arch/x86/include/asm/mpx.h renamed from arch/x86/include/asm/mpx.h~use-new-tsk_get_xsave_addr +4 -4
@@ -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 ; }
arch/x86/kernel/traps.c renamed from arch/x86/kernel/traps.c~use-new-tsk_get_xsave_addr +4 -6
@@ -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 =tsk_get_xsave_field (tsk,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
arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~use-new-tsk_get_xsave_addr +10 -11
@@ -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 ;
@@ -296,7 +296,7 @@ siginfo_t *mpx_generate_siginfo(struct p goto err_out; }/* get the bndregs _area_ of the xsave structure */ - bndregs =get_xsave_addr (xsave_buf,XSTATE_BNDREGS ); + bndregs =tsk_get_xsave_field (tsk,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 =tsk_get_xsave_field (tsk,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 =tsk_get_xsave_field (tsk,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_strif (!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
_
PATCH 3/16 x86, mpx: trace #BR exceptions
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
This is the first in a series of MPX tracing patches. I've found these extremely useful in the process of debugging applications and the kernel code itself.
This exception hooks in to the bounds (#BR) exception very early and allows capturing the key registers which would influence how the exception is handled.
Note that bndcfgu/bndstatus are technically still 64-bit registers even in 32-bit mode.
---
b/arch/x86/include/asm/trace/mpx.h | 48 +++++++++++++++++++++++++++++++++++++ b/arch/x86/kernel/traps.c | 2 + b/arch/x86/mm/mpx.c | 3 ++ 3 files changed, 53 insertions(+)
arch/x86/include/asm/trace/mpx.h new file +48 -0
@@ -0,0 +1,48 @@ +#undef TRACE_SYSTEM +#define TRACE_SYSTEM mpx + +#if !defined(_TRACE_MPX_H) || defined(TRACE_HEADER_MULTI_READ ) +#define _TRACE_MPX_H + +#include <linux/tracepoint.h> + +#ifdef CONFIG_X86_INTEL_MPX + +TRACE_EVENT (bounds_exception_mpx, + + TP_PROTO(struct bndcsr *bndcsr), +TP_ARGS (bndcsr), + +TP_STRUCT__entry ( + __field(u64, bndcfgu) +__field (u64, bndstatus) + ), + +TP_fast_assign ( + __entry->bndcfgu = bndcsr->bndcfgu; + __entry->bndstatus = bndcsr->bndstatus; + ), + +TP_printk ("bndcfgu:0x%llx bndstatus:0x%llx" , + __entry->bndcfgu, + __entry->bndstatus) +); + +#else + +/* +* This gets used outside of MPX-specific code, so we need a stub. +*/ +static inline void trace_bounds_exception_mpx (struct bndcsr *bndcsr) +{ +} + +#endif /* CONFIG_X86_INTEL_MPX */ + +#undef TRACE_INCLUDE_PATH +#define TRACE_INCLUDE_PATH asm/trace/ +#define TRACE_INCLUDE_FILE mpx +#endif /* _TRACE_MPX_H */ + +/* This part must be outside protection */ +#include <trace/define_trace.h>
arch/x86/kernel/traps.c renamed from arch/x86/kernel/traps.c~x86-br-exception-trace +2 -0
@@ -61,6 +61,7 @@#include <asm/mach_traps.h> #include <asm/alternative.h> #include <asm/mpx.h> +#include <asm/trace/mpx.h> #include <asm/xsave.h> #ifdef CONFIG_X86_64
@@ -401,6 +402,7 @@ dotraplinkage void do_bounds(struct pt_rif (!bndcsr) goto exit_trap; +trace_bounds_exception_mpx (bndcsr);/* * The error code field of the BNDSTATUS register communicates status * information of a bound range exception #BR or operation involving
arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~x86-br-exception-trace +3 -0
@@ -18,6 +18,9 @@#include <asm/processor.h> #include <asm/fpu-internal.h> +#define CREATE_TRACE_POINTS +#include <asm/trace/mpx.h> +static const char *mpx_mapping_name (struct vm_area_struct *vma) {return "[mpx]" ;
_
PATCH 4/16 x86, mpx: trace entry to bounds exception paths
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
There are two basic things that can happen as the result of a bounds exception (#BR):
1. We allocate a new bounds table
2. We pass up a bounds exception to userspace.
This patch adds a trace point for the case where we are passing the exception up to userspace with a signal.
We are also explicit that we're printing out the inverse of the 'upper' that we encounter. If you want to filter, for instance, you need to ~ the value first.
---
b/arch/x86/include/asm/trace/mpx.h | 25 +++++++++++++++++++++++++ b/arch/x86/mm/mpx.c | 2 ++ 2 files changed, 27 insertions(+)
arch/x86/include/asm/trace/mpx.h renamed from arch/x86/include/asm/trace/mpx.h~x86-mpx-trace-1 +25 -0
@@ -8,6 +8,31 @@#ifdef CONFIG_X86_INTEL_MPX +TRACE_EVENT (mpx_bounds_register_exception, + +TP_PROTO (void *addr_referenced, + struct bndreg *bndreg), +TP_ARGS (addr_referenced, bndreg), + +TP_STRUCT__entry ( + __field(void *, addr_referenced) + __field(u64, lower_bound) +__field (u64, upper_bound) + ), + +TP_fast_assign ( + __entry->addr_referenced = addr_referenced; + __entry->lower_bound = bndreg->lower_bound; + __entry->upper_bound = bndreg->upper_bound; + ), + +TP_printk ("address referenced: 0x%p bounds: lower: 0x%llx ~upper: 0x%llx" , + __entry->addr_referenced, + __entry->lower_bound, + ~__entry->upper_bound + ) +); + TRACE_EVENT(bounds_exception_mpx, TP_PROTO(struct bndcsr *bndcsr),
arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~x86-mpx-trace-1 +2 -0
@@ -16,6 +16,7 @@#include <asm/mmu_context.h> #include <asm/mpx.h> #include <asm/processor.h> +#include <asm/trace/mpx.h> #include <asm/fpu-internal.h> #define CREATE_TRACE_POINTS
@@ -337,6 +338,7 @@ siginfo_t *mpx_generate_siginfo(struct p err = -EINVAL ; goto err_out; } +trace_mpx_bounds_register_exception (info->si_addr, bndreg);return info; err_out:/* info might be NULL, but kfree() handles that */
_
PATCH 5/16 x86, mpx: trace ranged MPX operations
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
trace when MPX is zapping pages:
When MPX can not free an entire bounds table, it will instead try to zap unused parts of a bounds table to free the backing memory. This decreases RSS (resident set size) without decreasing the virtual space allocated for bounds tables.
trace attempts to find bounds tables:
This event traces any time we go looking to unmap a bounds table for a given virtual address range. This is useful to ensure that the kernel actually "tried" to free a bounds table versus times it succeeded.
It might try and fail if it realized that a table was shared with an adjacent VMA which is not being unmapped.
---
b/arch/x86/include/asm/trace/mpx.h | 32 ++++++++++++++++++++++++++++++++ b/arch/x86/mm/mpx.c | 2 ++ 2 files changed, 34 insertions(+)
arch/x86/include/asm/trace/mpx.h renamed from arch/x86/include/asm/trace/mpx.h~mpx-trace_unmap_zap +32 -0
@@ -53,6 +53,38 @@ TRACE_EVENT(bounds_exception_mpx, __entry->bndstatus) ); +DECLARE_EVENT_CLASS (mpx_range_trace, + + TP_PROTO(unsigned long start, + unsigned long end), +TP_ARGS (start, end), + +TP_STRUCT__entry ( + __field(unsigned long, start) + __field(unsigned long, end) + ), + +TP_fast_assign ( + __entry->start = start; + __entry->end = end; + ), + +TP_printk ("[0x%p:0x%p]" , + (void *)__entry->start, + (void *)__entry->end + ) +); + +DEFINE_EVENT (mpx_range_trace, mpx_unmap_zap, + TP_PROTO(unsigned long start, unsigned long end), +TP_ARGS (start, end) +); + +DEFINE_EVENT (mpx_range_trace, mpx_unmap_search, + TP_PROTO(unsigned long start, unsigned long end), +TP_ARGS (start, end) +); +#else /*
arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~mpx-trace_unmap_zap +2 -0
@@ -670,6 +670,7 @@ static int zap_bt_entries(struct mm_stru len =min (vma->vm_end, end) - addr;zap_page_range (vma, addr, len,NULL ); +trace_mpx_unmap_zap (addr, addr+len); vma = vma->vm_next; addr = vma->vm_start;
@@ -842,6 +843,7 @@ static int mpx_unmap_tables(struct mm_stlong __user *bd_entry, *bde_start, *bde_end;unsigned long bt_addr; +trace_mpx_unmap_search (start, end);/* * "Edge" bounds tables are those which are being used by the region * (start -> end), but that may be shared with adjacent areas. If they
_
PATCH 6/16 x86, mpx: trace allocation of new bounds tables
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
Bounds tables are a significant consumer of memory. It is important to know when they are being allocated. Add a trace point to trace whenever an allocation occurs and also its virtual address.
---
b/arch/x86/include/asm/trace/mpx.h | 16 ++++++++++++++++ b/arch/x86/mm/mpx.c | 1 + 2 files changed, 17 insertions(+)
arch/x86/include/asm/trace/mpx.h renamed from arch/x86/include/asm/trace/mpx.h~trace_mpx_new_bounds_table +16 -0
@@ -85,6 +85,22 @@ DEFINE_EVENT(mpx_range_trace, mpx_unmap_TP_ARGS (start, end) ); +TRACE_EVENT (mpx_new_bounds_table, + + TP_PROTO(unsigned long table_vaddr), +TP_ARGS (table_vaddr), + +TP_STRUCT__entry ( + __field(unsigned long, table_vaddr) + ), + +TP_fast_assign ( + __entry->table_vaddr = table_vaddr; + ), + +TP_printk ("table vaddr:%p" , (void *)__entry->table_vaddr) +); +#else /*
arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~trace_mpx_new_bounds_table +1 -0
@@ -485,6 +485,7 @@ static int allocate_bt(long __user *bd_e ret = -EINVAL ; goto out_unmap; } +trace_mpx_new_bounds_table (bt_addr);return 0 ; out_unmap:vm_munmap (bt_addr &MPX_BT_ADDR_MASK ,MPX_BT_SIZE_BYTES );
_
PATCH 7/16 x86, mpx: boot-time disable
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
MPX has the _potential_ to cause some issues. Say part of your init system tried to protect one of its components from buffer overflows with MPX. If there were a false positive, it's possible that MPX could keep a system from booting.
MPX could also potentially cause performance issues since it is present in hot paths like the unmap path.
Allow it to be disabled at boot time.
---
b/Documentation/kernel-parameters.txt | 4 ++++ b/arch/x86/kernel/cpu/common.c | 16 ++++++++++++++++ 2 files changed, 20 insertions(+)
arch/x86/kernel/cpu/common.c renamed from arch/x86/kernel/cpu/common.c~x86-mpx-disable-boot-time +16 -0
@@ -172,6 +172,22 @@ static int __init x86_xsaves_setup(char }__setup ("noxsaves" , x86_xsaves_setup); +static int __init x86_mpx_setup (char *s) +{ +/* require an exact match without trailing characters */ +if (strlen (s)) +return 0 ; + +/* do not emit a message if the feature is not present */ +if (!boot_cpu_has (X86_FEATURE_MPX )) +return 1 ; + +setup_clear_cpu_cap (X86_FEATURE_MPX ); +pr_info ("nompx: Intel Memory Protection Extensions (MPX) disabled\n" ); +return 1 ; +} +__setup ("nompx" , x86_mpx_setup); +#ifdef CONFIG_X86_32 static int cachesize_override =-1 ;static int disable_x86_serial_nr =1 ;
Documentation/kernel-parameters.txt renamed from Documentation/kernel-parameters.txt~x86-mpx-disable-boot-time +4 -0
@@ -2340,6 +2340,10 @@ bytes respectively. Such letter suffixes parameter, xsave area per process might occupy more memory on xsaves enabled systems. + nompx [X86] Disables Intel Memory Protection Extensions. + See Documentation/x86/intel_mpx.txt for more + information about the feature. + eagerfpu= [X86] on enable eager fpu restore off disable eager fpu restore
_
PATCH 8/16 x86: make is_64bit_mm() widely available
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
The uprobes code has a nice helper, is_64bit_mm(), that consults both the runtime and compile-time flags for 32-bit support. Instead of reinventing the wheel, pull it in to an x86 header so we can use it for MPX.
I prefer passing the mm around to test_thread_flag(TIF_IA32) because it makes it explicit where the context is coming from. It will also make it easier if we ever need to access the MPX structures from another process.
---
b/arch/x86/include/asm/mmu_context.h | 13 +++++++++++++ b/arch/x86/kernel/uprobes.c | 10 +--------- 2 files changed, 14 insertions(+), 9 deletions(-)
arch/x86/include/asm/mmu_context.h renamed from arch/x86/include/asm/mmu_context.h~x86-make-is_64bit_mm-available +13 -0
@@ -142,6 +142,19 @@ static inline void arch_exit_mmap(structparavirt_arch_exit_mmap (mm); } +#ifdef CONFIG_X86_64 +static inline bool is_64bit_mm (struct mm_struct *mm) +{ +return !config_enabled (CONFIG_IA32_EMULATION ) || + !(mm->context.ia32_compat ==TIF_IA32 ); +} +#else +static inline bool is_64bit_mm (struct mm_struct *mm) +{ +return false; +} +#endif +static inline void arch_bprm_mm_init (struct mm_struct *mm,struct vm_area_struct *vma) {
arch/x86/kernel/uprobes.c renamed from arch/x86/kernel/uprobes.c~x86-make-is_64bit_mm-available +1 -9
@@ -29,6 +29,7 @@#include <linux/kdebug.h> #include <asm/processor.h> #include <asm/insn.h> +#include <asm/mmu_context.h> /* Post-execution fixups. */
@@ -312,11 +313,6 @@ static int uprobe_init_insn(struct arch_ }#ifdef CONFIG_X86_64 -static inline bool is_64bit_mm (struct mm_struct *mm) -{ -return !config_enabled (CONFIG_IA32_EMULATION ) || - !(mm->context.ia32_compat ==TIF_IA32 ); -}/* * If arch_uprobe->insn doesn't use rip-relative addressing, return * immediately. Otherwise, rewrite the instruction so that it accesses
@@ -497,10 +493,6 @@ static void riprel_post_xol(struct arch_ } }#else /* 32-bit: */ -static inline bool is_64bit_mm (struct mm_struct *mm) -{ -return false; -}/* * No RIP-relative addressing on 32-bit */
_
PATCH 9/16 x86: make __VIRTUAL_MASK safe to use on 32 bit
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
We are going to do some calculations in a moment that are based on the size of the virtual address space. __VIRTUAL_MASK is currently unsafe to use on 32-bit since it overflows an unsigned long with its shift.
The current version will emit a warning if used at all on 32-bit kernels.
Add a version which is safe on 32-bit and consequently does not spit out a warning.
---
b/arch/x86/include/asm/page_types.h | 8 ++++++++ 1 file changed, 8 insertions(+)
arch/x86/include/asm/page_types.h renamed from arch/x86/include/asm/page_types.h~x86-make-__VIRTUAL_MASK-safe-to-use-on-32-bit +8 -0
@@ -10,7 +10,15 @@#define PAGE_MASK (~(PAGE_SIZE-1))#define __PHYSICAL_MASK ((phys_addr_t)((1ULL << __PHYSICAL_MASK_SHIFT) - 1)) +#ifdef CONFIG_X86_64 +/* +* This version doesn't work on 32-bit because __VIRTUAL_MASK_SHIFT=32 +* overflows the unsigned long we're trying to shift. +*/ #define __VIRTUAL_MASK ((1UL << __VIRTUAL_MASK_SHIFT) - 1) +#else +#define __VIRTUAL_MASK (~0UL) +#endif /* Cast PAGE_MASK to a signed type so that it is sign-extended if virtual addresses are 32-bits but physical addresses are larger
_
PATCH 10/16 x86, mpx: we do not allocate the bounds directory
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
The comment and code here are confusing. We do not currently allocate the bounds directory in the kernel.
---
b/arch/x86/mm/mpx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~x86-mpx-we-do-not-allocate-the-bounds-directory +2 -2
@@ -51,8 +51,8 @@ static unsigned long mpx_mmap(unsigned lvm_flags_t vm_flags;struct vm_area_struct *vma; -/* Only bounds table and bounds directory can be allocated here */ -if (len !=MPX_BD_SIZE_BYTES && len !=MPX_BT_SIZE_BYTES ) +/* Only bounds table can be allocated here */ +if (len !=MPX_BT_SIZE_BYTES )return -EINVAL ;down_write (&mm->mmap_sem);
_
PATCH 11/16 x86, mpx: remove redundant MPX_BNDCFG_ADDR_MASK
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
MPX_BNDCFG_ADDR_MASK is defined two times, so this patch removes redundant one.
---
b/arch/x86/include/asm/mpx.h | 1 - 1 file changed, 1 deletion(-)
arch/x86/include/asm/mpx.h renamed from arch/x86/include/asm/mpx.h~0001-x86-mpx-remove-redundant-MPX_BNDCFG_ADDR_MASK +0 -1
@@ -45,7 +45,6 @@#define MPX_BNDSTA_TAIL 2#define MPX_BNDCFG_TAIL 12#define MPX_BNDSTA_ADDR_MASK (~((1UL<<MPX_BNDSTA_TAIL)-1)) -#define MPX_BNDCFG_ADDR_MASK (~((1UL<<MPX_BNDCFG_TAIL)-1))#define MPX_BT_ADDR_MASK (~((1UL<<MPX_BD_ENTRY_TAIL)-1))#define MPX_BNDCFG_ADDR_MASK (~((1UL<<MPX_BNDCFG_TAIL)-1))
_
PATCH 12/16 x86, mpx: Add temporary variable to reduce masking
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
When we allocate a bounds table, we call mmap(), then add a "valid" bit to the value before storing it in to the bounds directory.
If we fail along the way, we go and mask that valid bit _back_ out. That seems a little silly, and this makes it much more clear when we have a plain address versus an actual table _entry_.
---
b/arch/x86/mm/mpx.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~mpx-remove-unnecessary-masking +4 -3
@@ -431,6 +431,7 @@ static int allocate_bt(long __user *bd_eunsigned long expected_old_val =0 ;unsigned long actual_old_val =0 ;unsigned long bt_addr; +unsigned long bd_new_entry;int ret =0 ;/*
@@ -443,7 +444,7 @@ static int allocate_bt(long __user *bd_e/* * Set the valid flag (kinda like _PAGE_PRESENT in a pte) */ - bt_addr = bt_addr |MPX_BD_ENTRY_VALID_FLAG ; + bd_new_entry = bt_addr |MPX_BD_ENTRY_VALID_FLAG ;/* * Go poke the address of the new bounds table in to the
@@ -457,7 +458,7 @@ static int allocate_bt(long __user *bd_e* of the MPX code that have to pagefault_disable(). */ ret =user_atomic_cmpxchg_inatomic (&actual_old_val, bd_entry, - expected_old_val, bt_addr); + expected_old_val, bd_new_entry);if (ret) goto out_unmap;
@@ -488,7 +489,7 @@ static int allocate_bt(long __user *bd_etrace_mpx_new_bounds_table (bt_addr);return 0 ; out_unmap: -vm_munmap (bt_addr &MPX_BT_ADDR_MASK ,MPX_BT_SIZE_BYTES ); +vm_munmap (bt_addr,MPX_BT_SIZE_BYTES );return ret; }
_
PATCH 13/16 x86, mpx: new directory entry to addr helper
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
Currently, to get from a bounds directory entry to the virtual address of a bounds table, we simply mask off a few low bits. However, the set of bits we mask off is different for 32 and 64-bit binaries.
This breaks the operation out in to a helper function and also adds a temporary variable to store the result until we are sure we are returning one.
---
b/arch/x86/include/asm/mpx.h | 1 - b/arch/x86/mm/mpx.c | 41 ++++++++++++++++++++++++++++++++++------- 2 files changed, 34 insertions(+), 8 deletions(-)
arch/x86/include/asm/mpx.h renamed from arch/x86/include/asm/mpx.h~mpx-new-entry-to-addr-helper +0 -1
@@ -45,7 +45,6 @@#define MPX_BNDSTA_TAIL 2#define MPX_BNDCFG_TAIL 12#define MPX_BNDSTA_ADDR_MASK (~((1UL<<MPX_BNDSTA_TAIL)-1)) -#define MPX_BT_ADDR_MASK (~((1UL<<MPX_BD_ENTRY_TAIL)-1))#define MPX_BNDCFG_ADDR_MASK (~((1UL<<MPX_BNDCFG_TAIL)-1))#define MPX_BNDSTA_ERROR_CODE 0x3
arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~mpx-new-entry-to-addr-helper +34 -7
84 lines not on this page · raw message
_
PATCH 14/16 x86, mpx: do 32-bit-only cmpxchg for 32-bit apps
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
user_atomic_cmpxchg_inatomic() actually looks at sizeof(*ptr) to figure out how many bytes to copy. If we run it on a 64-bit kernel with a 64-bit pointer, it will copy a 64-bit bounds directory entry. That's fine, except when we have 32-bit programs with 32-bit bounds directory entries and we only *want* 32-bits.
This patch breaks the cmpxchg operation out in to its own function and performs the 32-bit type swizzling in there.
Note, the "64-bit" version of this code _would_ work on a 32-bit-only kernel. The issue this patch addresses is only for when the kernel's 'long' is mismatched from the size of the bounds directory entry of the process we are working on.
The new helper modifies 'actual_old_val' or returns an error. But gcc doesn't know this, so it warns about 'actual_old_val' being unused. Shut it up with an uninitialized_var().
---
b/arch/x86/mm/mpx.c | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-)
arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~mpx-variable-sized-userspace-pokes +36 -5
@@ -421,6 +421,35 @@ int mpx_disable_management(struct task_sreturn 0 ; } +static int mpx_cmpxchg_bd_entry (struct mm_struct *mm, +unsigned long *actual_old_val_ptr,long __user *bd_entry_addr, +unsigned long expected_old_val,unsigned long new_bd_entry) +{ +int ret; +/* +* user_atomic_cmpxchg_inatomic() actually uses sizeof() +* the pointer that we pass to it to figure out how much +* data to cmpxchg. We have to be careful here not to +* pass a pointer to a 64-bit data type when we only want +* a 32-bit copy. +*/ +if (is_64bit_mm (mm)) { + ret =user_atomic_cmpxchg_inatomic (actual_old_val_ptr, + bd_entry_addr, expected_old_val, new_bd_entry); + }else { +u32 uninitialized_var (actual_old_val_32 ); +u32 expected_old_val_32 = expected_old_val; +u32 new_bd_entry_32 = new_bd_entry; +u32 __user *bd_entry_32 = (u32 __user *)bd_entry_addr; + ret =user_atomic_cmpxchg_inatomic (&actual_old_val_32, + bd_entry_32, expected_old_val_32, + new_bd_entry_32); +if (!ret) + *actual_old_val_ptr = actual_old_val_32; + } +return ret; +} +/* * With 32-bit mode, MPX_BT_SIZE_BYTES is 4MB, and the size of each * bounds table is 16KB. With 64-bit mode, MPX_BT_SIZE_BYTES is 2GB,
@@ -428,6 +457,7 @@ int mpx_disable_management(struct task_s*/ static int allocate_bt (long __user *bd_entry) { +struct mm_struct *mm = current->mm;unsigned long expected_old_val =0 ;unsigned long actual_old_val =0 ;unsigned long bt_addr;
@@ -457,8 +487,8 @@ static int allocate_bt(long __user *bd_e* mmap_sem at this point, unlike some of the other part * of the MPX code that have to pagefault_disable(). */ - ret =user_atomic_cmpxchg_inatomic (&actual_old_val, bd_entry, - expected_old_val, bd_new_entry); + ret =mpx_cmpxchg_bd_entry (mm, &actual_old_val, bd_entry, + expected_old_val, bd_new_entry);if (ret) goto out_unmap;
@@ -712,15 +742,16 @@ static int unmap_single_bt(struct mm_str long __user *bd_entry, unsigned long bt_addr) {unsigned long expected_old_val = bt_addr |MPX_BD_ENTRY_VALID_FLAG ; -unsigned long actual_old_val =0 ; +unsigned long uninitialized_var (actual_old_val );int ret;while (1 ) {int need_write =1 ; +unsigned long cleared_bd_entry =0 ;pagefault_disable (); - ret =user_atomic_cmpxchg_inatomic (&actual_old_val, bd_entry, - expected_old_val,0 ); + ret =mpx_cmpxchg_bd_entry (mm, &actual_old_val, + bd_entry, bt_addr, cleared_bd_entry);pagefault_enable ();if (!ret)break ;
_
PATCH 15/16 x86, mpx: support 32-bit binaries on 64-bit kernel
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
Right now, the kernel can only switch between 64-bit and 32-bit binaries at compile time. This patch adds support for 32-bit binaries on 64-bit kernels when we support ia32 emulation.
We essentially choose which set of table sizes to use when doing arithmetic for the bounds table calculations.
This also uses a different approach for calculating the table indexes than before. I think the new one makes it much more clear what is going on, and allows us to share more code between the 32 and 64-bit cases.
---
b/arch/x86/include/asm/mpx.h | 68 +++++++++---------- b/arch/x86/mm/mpx.c | 150 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 163 insertions(+), 55 deletions(-)
arch/x86/include/asm/mpx.h renamed from arch/x86/include/asm/mpx.h~0002-x86-mpx-support-32bit-binaries-on-64bit-kernel +34 -34
@@ -13,49 +13,49 @@#define MPX_BNDCFG_ENABLE_FLAG 0x1#define MPX_BD_ENTRY_VALID_FLAG 0x1 -#ifdef CONFIG_X86_64 - -/* upper 28 bits [47:20] of the virtual address in 64-bit used to -* index into bounds directory (BD). +/* +* The upper 28 bits [47:20] of the virtual address in 64-bit +* are used to index into bounds directory (BD). +* +* The directory is 2G (2^31) in size, and with 8-byte entries +* it has 2^28 entries. */ -#define MPX_BD_ENTRY_OFFSET 28 -#define MPX_BD_ENTRY_SHIFT 3 -/* bits [19:3] of the virtual address in 64-bit used to index into -* bounds table (BT). +#define MPX_BD_SIZE_BYTES_64 (1UL<<31) +/* An entry is a long, so 8 bytes and a shift of 3 */ +#define MPX_BD_ENTRY_BYTES_64 8 +#define MPX_BD_NR_ENTRIES_64 (MPX_BD_SIZE_BYTES_64/MPX_BD_ENTRY_BYTES_64) + +/* +* The 32-bit directory is 4MB (2^22) in size, and with 4-byte +* entries it has 2^20 entries. */ -#define MPX_BT_ENTRY_OFFSET 17 -#define MPX_BT_ENTRY_SHIFT 5 -#define MPX_IGN_BITS 3 -#define MPX_BD_ENTRY_TAIL 3 - -#else - -#define MPX_BD_ENTRY_OFFSET 20 -#define MPX_BD_ENTRY_SHIFT 2 -#define MPX_BT_ENTRY_OFFSET 10 -#define MPX_BT_ENTRY_SHIFT 4 -#define MPX_IGN_BITS 2 -#define MPX_BD_ENTRY_TAIL 2 - -#endif - -#define MPX_BD_SIZE_BYTES (1UL<<(MPX_BD_ENTRY_OFFSET+MPX_BD_ENTRY_SHIFT)) -#define MPX_BT_SIZE_BYTES (1UL<<(MPX_BT_ENTRY_OFFSET+MPX_BT_ENTRY_SHIFT)) +#define MPX_BD_SIZE_BYTES_32 (1UL<<22) +/* An entry is a long, so 4 bytes and a shift of 2 */ +#define MPX_BD_ENTRY_BYTES_32 4 +#define MPX_BD_NR_ENTRIES_32 (MPX_BD_SIZE_BYTES_32/MPX_BD_ENTRY_BYTES_32) + +/* +* A 64-bit table is 4MB total in size, and an entry is +* 4 64-bit pointers in size. +*/ +#define MPX_BT_SIZE_BYTES_64 (1UL<<22) +#define MPX_BT_ENTRY_BYTES_64 32 +#define MPX_BT_NR_ENTRIES_64 (MPX_BD_SIZE_BYTES_64/MPX_BD_ENTRY_BYTES_64) + +/* +* A 32-bit table is 16kB total in size, and an entry is +* 4 32-bit pointers in size. +*/ +#define MPX_BT_SIZE_BYTES_32 (1UL<<14) +#define MPX_BT_ENTRY_BYTES_32 16 +#define MPX_BT_NR_ENTRIES_32 (MPX_BD_SIZE_BYTES_32/MPX_BD_ENTRY_BYTES_32)#define MPX_BNDSTA_TAIL 2#define MPX_BNDCFG_TAIL 12#define MPX_BNDSTA_ADDR_MASK (~((1UL<<MPX_BNDSTA_TAIL)-1)) -#define MPX_BNDCFG_ADDR_MASK (~((1UL<<MPX_BNDCFG_TAIL)-1))#define MPX_BNDSTA_ERROR_CODE 0x3 -#define MPX_BD_ENTRY_MASK ((1<<MPX_BD_ENTRY_OFFSET)-1) -#define MPX_BT_ENTRY_MASK ((1<<MPX_BT_ENTRY_OFFSET)-1) -#define MPX_GET_BD_ENTRY_OFFSET (addr) ((((addr)>>(MPX_BT_ENTRY_OFFSET+ \ - MPX_IGN_BITS)) & MPX_BD_ENTRY_MASK) << MPX_BD_ENTRY_SHIFT) -#define MPX_GET_BT_ENTRY_OFFSET (addr) ((((addr)>>MPX_IGN_BITS) & \ - MPX_BT_ENTRY_MASK) << MPX_BT_ENTRY_SHIFT) -#ifdef CONFIG_X86_INTEL_MPX siginfo_t *mpx_generate_siginfo (struct pt_regs *regs,struct task_struct *tsk);
arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~0002-x86-mpx-support-32bit-binaries-on-64bit-kernel +129 -21
241 lines not on this page · raw message
_
PATCH 16/16 x86, mpx: allow mixed binaries again
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
We explicitly disable allowing 32-bit binaries to enable MPX on 64-bit kernels. Re-allow that.
---
b/arch/x86/mm/mpx.c | 6 ------ 1 file changed, 6 deletions(-)
arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~x86-mpx-allow-mixed-binaries-again +0 -6
@@ -370,12 +370,6 @@ static __user void *task_get_bounds_dir(return MPX_INVALID_BOUNDS_DIR ;/* -* 32-bit binaries on 64-bit kernels are currently -* unsupported. -*/ -if (IS_ENABLED (CONFIG_X86_64 ) &&test_thread_flag (TIF_IA32 )) -return MPX_INVALID_BOUNDS_DIR ; -/* * The bounds directory pointer is stored in a register * only accessible if we first do an xsave. */
_
Quoted 7 lines · Dave Hansen
Hi x86 maintainers,
There are 4 basic things going on here:
1. Make FPU/xsave code preempt safe and work properly
2. Add trace points to make kernel and app debugging easier
3. Add a boot-time disable for mpx
4. Support 32-bit binaries to run on 64-bit kernels
Even though this missed 4.1, I'd still appreciate it being picked up for the 4.2 cycle if there are no new issues with it.
Borislav Petkov
maintainerQuoted 1 line · Dave Hansen
Changes from take 3 / v18 (all minor):
Btw, do you have a test program for the MPX stuff I could use?
Thanks.
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Quoted 3 lines · Borislav Petkov
Quoted 1 line · Dave Hansen
Quoted 1 line
Changes from take 3 / v18 (all minor):
Btw, do you have a test program for the MPX stuff I could use?
GCC 5.0 contains the necessary support to generate MPX instructions, so you can get Fedora 21 (needed for the libc support), gcc 5 and start compiling and running things with the MPX flags.
But do you have any hardware that supports it?
I do have another standalone test program, but it's not something I can share at the moment.