[patch] x64: fix FPU corruption with signals and preemption

[patch] x64: fix FPU corruption with signals and preemption

[Suresh Siddha]

From: Suresh Siddha <suresh.b.siddha@intel.com>
Subject: x64: fix FPU corruption with signals and preemption

Impact: fix FPU state corruption

In 64bit signal delivery path, clear_used_math() was happening before saving
the current active FPU state on to the user stack for signal handling. Between
clear_used_math() and the state store on to the user stack, potentially we
can get a page fault for the user address and can block. Infact, while testing
we were hitting the might_fault() in __clear_user() which can do a schedule().

At a later point in time, we will schedule back into this process and
resume the save state (using "xsave/fxsave" instruction) which can lead
to DNA fault. And as used_math was cleared before, we will reinit the FP state
in the DNA fault and continue. This reinit will result in loosing the
FPU state of the process.

Move clear_used_math() to a point after the FPU state has been stored
onto the user stack.

This issue is present from a long time (even before the xsave changes
and the x86 merge). But it can easily be exposed in 2.6.28.x and 2.6.29.x
series because of the __clear_user() in this path, which has an explicit
__cond_resched() leading to a context switch with CONFIG_PREEMPT_VOLUNTARY.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: stable@kernel.org			[2.6.28.x, 2.6.29.x]
---

Index: tip/arch/x86/kernel/xsave.c
===================================================================
--- tip.orig/arch/x86/kernel/xsave.c
+++ tip/arch/x86/kernel/xsave.c
@@ -89,7 +89,7 @@ int save_i387_xstate(void __user *buf)
 
 	if (!used_math())
 		return 0;
-	clear_used_math(); /* trigger finit */
+
 	if (task_thread_info(tsk)->status & TS_USEDFPU) {
 		/*
 	 	 * Start with clearing the user buffer. This will present a
@@ -114,6 +114,8 @@ int save_i387_xstate(void __user *buf)
 			return -1;
 	}
 
+	clear_used_math(); /* trigger finit */
+
 	if (task_thread_info(tsk)->status & TS_XSAVE) {
 		struct _fpstate __user *fx = buf;
 		struct _xstate __user *x = buf;

Re: [stable] [patch] x64: fix FPU corruption with signals and preemption

[Chris Wright]

* Suresh Siddha (suresh.b.siddha@intel.com) wrote:
> From: Suresh Siddha <suresh.b.siddha@intel.com>
> Subject: x64: fix FPU corruption with signals and preemption
> 
> Impact: fix FPU state corruption
> 
> In 64bit signal delivery path, clear_used_math() was happening before saving
> the current active FPU state on to the user stack for signal handling. Between
> clear_used_math() and the state store on to the user stack, potentially we
> can get a page fault for the user address and can block. Infact, while testing
> we were hitting the might_fault() in __clear_user() which can do a schedule().
> 
> At a later point in time, we will schedule back into this process and
> resume the save state (using "xsave/fxsave" instruction) which can lead
> to DNA fault. And as used_math was cleared before, we will reinit the FP state
> in the DNA fault and continue. This reinit will result in loosing the
> FPU state of the process.
> 
> Move clear_used_math() to a point after the FPU state has been stored
> onto the user stack.
> 
> This issue is present from a long time (even before the xsave changes
> and the x86 merge). But it can easily be exposed in 2.6.28.x and 2.6.29.x
> series because of the __clear_user() in this path, which has an explicit
> __cond_resched() leading to a context switch with CONFIG_PREEMPT_VOLUNTARY.
> 
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: stable@kernel.org			[2.6.28.x, 2.6.29.x]

This one get lost?

thanks,
-chris

Re: [stable] [patch] x64: fix FPU corruption with signals and preemption

[H. Peter Anvin]

Chris Wright wrote:
>>
>> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
>> Cc: stable@kernel.org			[2.6.28.x, 2.6.29.x]
> 
> This one get lost?
> 

No, it's queued up in tip:x86/urgent and will be pushed upstream with 
the next push of x86 fixes to Linus.

	-hpa

Re: [stable] [patch] x64: fix FPU corruption with signals and preemption

[Chris Wright]

* H. Peter Anvin (hpa@linux.intel.com) wrote:
> Chris Wright wrote:
>>>
>>> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
>>> Cc: stable@kernel.org			[2.6.28.x, 2.6.29.x]
>>
>> This one get lost?
>
> No, it's queued up in tip:x86/urgent and will be pushed upstream with  
> the next push of x86 fixes to Linus.

OK, I had it marked as not upstream and not in tip, thanks for the
correction.

thanks,
-chris

Re: [stable] [patch] x64: fix FPU corruption with signals and preemption

[Ingo Molnar]

* Chris Wright <chrisw@sous-sol.org> wrote:

> * H. Peter Anvin (hpa@linux.intel.com) wrote:
> > Chris Wright wrote:
> >>>
> >>> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> >>> Cc: stable@kernel.org			[2.6.28.x, 2.6.29.x]
> >>
> >> This one get lost?
> >
> > No, it's queued up in tip:x86/urgent and will be pushed upstream with  
> > the next push of x86 fixes to Linus.
> 
> OK, I had it marked as not upstream and not in tip, thanks for the 
> correction.

there was a bit of a patch delay with it. Since this is an old race 
leading back to ancient times, and because this is touching critical 
code, we kept it around some more before pushing it to Linus. The 
plan is for it to show up in -git in the next few days and then in 
-rc4.

	Ingo