[lkml]   [2012]   [Feb]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    Subject[ 04/10] i387: fix x86-64 preemption-unsafe user stack save/restore
    3.2-stable review patch.  If anyone has any objections, please let me know.


    From: Linus Torvalds <>

    commit 15d8791cae75dca27bfda8ecfe87dca9379d6bb0 upstream.

    Commit 5b1cbac37798 ("i387: make irq_fpu_usable() tests more robust")
    added a sanity check to the #NM handler to verify that we never cause
    the "Device Not Available" exception in kernel mode.

    However, that check actually pinpointed a (fundamental) race where we do
    cause that exception as part of the signal stack FPU state save/restore

    Because we use the floating point instructions themselves to save and
    restore state directly from user mode, we cannot do that atomically with
    testing the TS_USEDFPU bit: the user mode access itself may cause a page
    fault, which causes a task switch, which saves and restores the FP/MMX
    state from the kernel buffers.

    This kind of "recursive" FP state save is fine per se, but it means that
    when the signal stack save/restore gets restarted, it will now take the
    '#NM' exception we originally tried to avoid. With preemption this can
    happen even without the page fault - but because of the user access, we
    cannot just disable preemption around the save/restore instruction.

    There are various ways to solve this, including using the
    "enable/disable_page_fault()" helpers to not allow page faults at all
    during the sequence, and fall back to copying things by hand without the
    use of the native FP state save/restore instructions.

    However, the simplest thing to do is to just allow the #NM from kernel
    space, but fix the race in setting and clearing CR0.TS that this all
    exposed: the TS bit changes and the TS_USEDFPU bit absolutely have to be
    atomic wrt scheduling, so while the actual state save/restore can be
    interrupted and restarted, the act of actually clearing/setting CR0.TS
    and the TS_USEDFPU bit together must not.

    Instead of just adding random "preempt_disable/enable()" calls to what
    is already excessively ugly code, this introduces some helper functions
    that mostly mirror the "kernel_fpu_begin/end()" functionality, just for
    the user state instead.

    Those helper functions should probably eventually replace the other
    ad-hoc CR0.TS and TS_USEDFPU tests too, but I'll need to think about it
    some more: the task switching functionality in particular needs to
    expose the difference between the 'prev' and 'next' threads, while the
    new helper functions intentionally were written to only work with

    Signed-off-by: Linus Torvalds <>
    Signed-off-by: Greg Kroah-Hartman <>

    arch/x86/include/asm/i387.h | 42 ++++++++++++++++++++++++++++++++++++++++++
    arch/x86/kernel/traps.c | 1 -
    arch/x86/kernel/xsave.c | 10 +++-------
    3 files changed, 45 insertions(+), 8 deletions(-)

    --- a/arch/x86/include/asm/i387.h
    +++ b/arch/x86/include/asm/i387.h
    @@ -400,6 +400,48 @@ static inline void irq_ts_restore(int TS

    + * The question "does this thread have fpu access?"
    + * is slightly racy, since preemption could come in
    + * and revoke it immediately after the test.
    + *
    + * However, even in that very unlikely scenario,
    + * we can just assume we have FPU access - typically
    + * to save the FP state - we'll just take a #NM
    + * fault and get the FPU access back.
    + *
    + * The actual user_fpu_begin/end() functions
    + * need to be preemption-safe, though.
    + *
    + * NOTE! user_fpu_end() must be used only after you
    + * have saved the FP state, and user_fpu_begin() must
    + * be used only immediately before restoring it.
    + * These functions do not do any save/restore on
    + * their own.
    + */
    +static inline int user_has_fpu(void)
    + return current_thread_info()->status & TS_USEDFPU;
    +static inline void user_fpu_end(void)
    + preempt_disable();
    + current_thread_info()->status &= ~TS_USEDFPU;
    + stts();
    + preempt_enable();
    +static inline void user_fpu_begin(void)
    + preempt_disable();
    + if (!user_has_fpu()) {
    + clts();
    + current_thread_info()->status |= TS_USEDFPU;
    + }
    + preempt_enable();
    * These disable preemption on their own and are safe
    static inline void save_init_fpu(struct task_struct *tsk)
    --- a/arch/x86/kernel/traps.c
    +++ b/arch/x86/kernel/traps.c
    @@ -622,7 +622,6 @@ EXPORT_SYMBOL_GPL(math_state_restore);
    dotraplinkage void __kprobes
    do_device_not_available(struct pt_regs *regs, long error_code)
    - WARN_ON_ONCE(!user_mode_vm(regs));
    if (read_cr0() & X86_CR0_EM) {
    struct math_emu_info info = { };
    --- a/arch/x86/kernel/xsave.c
    +++ b/arch/x86/kernel/xsave.c
    @@ -168,7 +168,7 @@ int save_i387_xstate(void __user *buf)
    if (!used_math())
    return 0;

    - if (task_thread_info(tsk)->status & TS_USEDFPU) {
    + if (user_has_fpu()) {
    if (use_xsave())
    err = xsave_user(buf);
    @@ -176,8 +176,7 @@ int save_i387_xstate(void __user *buf)

    if (err)
    return err;
    - task_thread_info(tsk)->status &= ~TS_USEDFPU;
    - stts();
    + user_fpu_end();
    } else {
    if (__copy_to_user(buf, &tsk->thread.fpu.state->fxsave,
    @@ -292,10 +291,7 @@ int restore_i387_xstate(void __user *buf
    return err;

    - if (!(task_thread_info(current)->status & TS_USEDFPU)) {
    - clts();
    - task_thread_info(current)->status |= TS_USEDFPU;
    - }
    + user_fpu_begin();
    if (use_xsave())
    err = restore_user_xstate(buf);

     \ /
      Last update: 2012-02-24 01:09    [W:0.026 / U:2.780 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site