From: tip-bot for Dave Hansen <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: sbsiddha@gmail.com, peterz@infradead.org,
torvalds@linux-foundation.org, luto@amacapital.net,
linux-kernel@vger.kernel.org, dave@sr71.net, oleg@redhat.com,
hpa@zytor.com, fenghua.yu@intel.com, riel@redhat.com,
mingo@kernel.org, dave.hansen@linux.intel.com,
tglx@linutronix.de, akpm@linux-foundation.org
Subject: [tip:x86/fpu] x86/fpu/xstate: Wrap get_xsave_addr() to make it safer
Date: Tue, 9 Jun 2015 05:31:13 -0700 [thread overview]
Message-ID: <tip-04cd027bcba1ead7bfe39e7f1c6f4d993c4c3323@git.kernel.org> (raw)
In-Reply-To: <20150607183700.AA881696@viggo.jf.intel.com>
Commit-ID: 04cd027bcba1ead7bfe39e7f1c6f4d993c4c3323
Gitweb: http://git.kernel.org/tip/04cd027bcba1ead7bfe39e7f1c6f4d993c4c3323
Author: Dave Hansen <dave.hansen@linux.intel.com>
AuthorDate: Sun, 7 Jun 2015 11:37:00 -0700
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 9 Jun 2015 12:24:29 +0200
x86/fpu/xstate: Wrap get_xsave_addr() to make it safer
The MPX code appears is calling a low-level FPU function
(copy_fpregs_to_fpstate()). This function is not able to
be called in all contexts, although it is safe to call
directly in some cases.
Although probably correct, the current code is ugly and
potentially error-prone. So, add a wrapper that calls
the (slightly) higher-level fpu__save() (which is preempt-
safe) and also ensures that we even *have* an FPU context
(in the case that this was called when in lazy FPU mode).
Ingo had this to say about the details about when we need
preemption disabled:
> it's indeed generally unsafe to access/copy FPU registers with preemption enabled,
> for two reasons:
>
> - on older systems that use FSAVE the instruction destroys FPU register
> contents, which has to be handled carefully
>
> - even on newer systems if we copy to FPU registers (which this code doesn't)
> then we don't want a context switch to occur in the middle of it, because a
> context switch will write to the fpstate, potentially overwriting our new data
> with old FPU state.
>
> But it's safe to access FPU registers with preemption enabled in a couple of
> special cases:
>
> - potentially destructively saving FPU registers: the signal handling code does
> this in copy_fpstate_to_sigframe(), because it can rely on the signal restore
> side to restore the original FPU state.
>
> - reading FPU registers on modern systems: we don't do this anywhere at the
> moment, mostly to keep symmetry with older systems where FSAVE is
> destructive.
>
> - initializing FPU registers on modern systems: fpu__clear() does this. Here
> it's safe because we don't copy from the fpstate.
>
> - directly writing FPU registers from user-space memory (!). We do this in
> fpu__restore_sig(), and it's safe because neither context switches nor
> irq-handler FPU use can corrupt the source context of the copy (which is
> user-space memory).
>
> Note that the MPX code's current use of copy_fpregs_to_fpstate() was safe I think,
> because:
>
> - MPX is predicated on eagerfpu, so the destructive F[N]SAVE instruction won't be
> used.
>
> - the code was only reading FPU registers, and was doing it only in places that
> guaranteed that an FPU state was already active (i.e. didn't do it in
> kthreads)
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Hansen <dave@sr71.net>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: bp@alien8.de
Link: http://lkml.kernel.org/r/20150607183700.AA881696@viggo.jf.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/fpu/xstate.h | 1 +
arch/x86/kernel/fpu/xstate.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 3398946..4656b25 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -41,5 +41,6 @@ extern u64 xstate_fx_sw_bytes[USER_XSTATE_FX_SW_WORDS];
extern void update_regset_xstate_info(unsigned int size, u64 xstate_mask);
void *get_xsave_addr(struct xregs_state *xsave, int xstate);
+const void *get_xsave_field_ptr(int xstate_field);
#endif
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index af3700e..49d0d9b 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -427,3 +427,35 @@ void *get_xsave_addr(struct xregs_state *xsave, int xstate_feature)
return (void *)xsave + xstate_comp_offsets[feature_nr];
}
EXPORT_SYMBOL_GPL(get_xsave_addr);
+
+/*
+ * This wraps up the common operations that need to occur when retrieving
+ * data from xsave state. It first ensures that the current task was
+ * 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.
+ *
+ * Note that this only works on the current task.
+ *
+ * Inputs:
+ * @xsave_state: state which is defined in xsave.h (e.g. XSTATE_FP,
+ * XSTATE_SSE, etc...)
+ * Output:
+ * address of the state in the xsave area or NULL if the state
+ * is not present or is in its 'init state'.
+ */
+const void *get_xsave_field_ptr(int xsave_state)
+{
+ struct fpu *fpu = ¤t->thread.fpu;
+
+ if (!fpu->fpstate_active)
+ return NULL;
+ /*
+ * fpu__save() takes the CPU's xstate registers
+ * and saves them off to the 'fpu memory buffer.
+ */
+ fpu__save(fpu);
+
+ return get_xsave_addr(&fpu->state.xsave, xsave_state);
+}
next prev parent reply other threads:[~2015-06-09 12:35 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-07 18:37 [PATCH 00/19] x86, mpx updates for 4.2 (take 9) Dave Hansen
2015-06-07 18:37 ` [PATCH 01/19] x86, mpx, xsave: Fix up bad get_xsave_addr() assumptions Dave Hansen
2015-06-09 12:30 ` [tip:x86/fpu] x86/fpu/xstate: " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 02/19] x86, fpu: Wrap get_xsave_addr() to make it safer Dave Hansen
2015-06-09 12:31 ` tip-bot for Dave Hansen [this message]
2015-06-07 18:37 ` [PATCH 03/19] x86, mpx: Use new get_xsave_field_ptr() Dave Hansen
2015-06-09 12:31 ` [tip:x86/fpu] x86/mpx: Use the new get_xsave_field_ptr()API tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 06/19] x86, mpx: Restrict mmap size check to bounds tables Dave Hansen
2015-06-09 12:32 ` [tip:x86/fpu] x86/mpx: Restrict the mmap() " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 05/19] x86, mpx: remove redundant MPX_BNDCFG_ADDR_MASK Dave Hansen
2015-06-09 12:32 ` [tip:x86/fpu] x86/mpx: Remove " tip-bot for Qiaowei Ren
2015-06-07 18:37 ` [PATCH 04/19] x86, mpx: Cleanup: Do not pass task around when unnecessary Dave Hansen
2015-06-09 12:31 ` [tip:x86/fpu] x86/mpx: Clean up the code by not passing a task pointer " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 07/19] x86, mpx: boot-time disable Dave Hansen
2015-06-09 12:32 ` [tip:x86/fpu] x86/mpx: Introduce a boot-time disable flag tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 09/19] x86, mpx: trace entry to bounds exception paths Dave Hansen
2015-06-09 12:33 ` [tip:x86/fpu] x86/mpx: Trace " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 10/19] x86, mpx: Trace the attempts to find bounds tables Dave Hansen
2015-06-09 12:33 ` [tip:x86/fpu] x86/mpx: " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 08/19] x86, mpx: trace #BR exceptions Dave Hansen
2015-06-09 12:33 ` [tip:x86/fpu] x86/mpx: Trace " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 12/19] x86: make is_64bit_mm() widely available Dave Hansen
2015-06-09 12:34 ` [tip:x86/fpu] x86: Make " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 13/19] x86, mpx: Add temporary variable to reduce masking Dave Hansen
2015-06-09 12:34 ` [tip:x86/fpu] x86/mpx: " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 11/19] x86, mpx: trace allocation of new bounds tables Dave Hansen
2015-06-09 12:33 ` [tip:x86/fpu] x86/mpx: Trace " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 14/19] x86, mpx: new directory entry to addr helper Dave Hansen
2015-06-09 12:34 ` [tip:x86/fpu] x86/mpx: Introduce new 'directory entry' to 'addr' helper function tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 15/19] x86, mpx: do 32-bit-only cmpxchg for 32-bit apps Dave Hansen
2015-06-09 12:35 ` [tip:x86/fpu] x86/mpx: Use 32-bit-only cmpxchg() " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 16/19] x86, mpx: support 32-bit binaries on 64-bit kernel Dave Hansen
2015-06-09 12:35 ` [tip:x86/fpu] x86/mpx: Support 32-bit binaries on 64-bit kernels tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 17/19] x86, mpx: rewrite unmap code Dave Hansen
2015-06-09 12:35 ` [tip:x86/fpu] x86/mpx: Rewrite the " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 18/19] x86, mpx: do not count MPX VMAs as neighbors when unmapping Dave Hansen
2015-06-09 10:23 ` Ingo Molnar
2015-06-09 12:35 ` [tip:x86/fpu] x86/mpx: Do " tip-bot for Dave Hansen
2015-06-07 18:37 ` [PATCH 19/19] x86, mpx: allow mixed binaries again Dave Hansen
2015-06-09 12:36 ` [tip:x86/fpu] x86/mpx: Allow 32-bit binaries on 64-bit kernels again tip-bot for Dave Hansen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=tip-04cd027bcba1ead7bfe39e7f1c6f4d993c4c3323@git.kernel.org \
--to=tipbot@zytor.com \
--cc=akpm@linux-foundation.org \
--cc=dave.hansen@linux.intel.com \
--cc=dave@sr71.net \
--cc=fenghua.yu@intel.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tip-commits@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mingo@kernel.org \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=riel@redhat.com \
--cc=sbsiddha@gmail.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox