• Dave Hansen's avatar
    x86/fpu/xstate: Wrap get_xsave_addr() to make it safer · 04cd027b
    Dave Hansen authored
    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: default avatarDave Hansen <dave.hansen@linux.intel.com>
    Reviewed-by: default avatarThomas 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.comSigned-off-by: default avatarIngo Molnar <mingo@kernel.org>
    04cd027b
xstate.h 1.22 KB