From: Suresh Siddha <[email protected]>
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 <[email protected]>
Cc: [email protected] [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;
* Suresh Siddha ([email protected]) wrote:
> From: Suresh Siddha <[email protected]>
> 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 <[email protected]>
> Cc: [email protected] [2.6.28.x, 2.6.29.x]
This one get lost?
thanks,
-chris
Chris Wright wrote:
>>
>> Signed-off-by: Suresh Siddha <[email protected]>
>> Cc: [email protected] [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
* H. Peter Anvin ([email protected]) wrote:
> Chris Wright wrote:
>>>
>>> Signed-off-by: Suresh Siddha <[email protected]>
>>> Cc: [email protected] [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
* Chris Wright <[email protected]> wrote:
> * H. Peter Anvin ([email protected]) wrote:
> > Chris Wright wrote:
> >>>
> >>> Signed-off-by: Suresh Siddha <[email protected]>
> >>> Cc: [email protected] [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