[PATCH] Re: SSE related security hole

Brian Gerst
2002-04-19T23:14:31Z

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

H. Peter Anvin wrote:
>>> Ummm...last I knew, fxrstor is *expensive*. The fninit/xor regs
>>> setup is likely *very* much faster. Someone should check this
>>> before we sacrifice 100 cycles needlessly or something.
>>
>> most probably yes, fxrestor needs to read ram, pxor also takes
>> some icache and bytecode ram but it sounds like it will be faster.
>>
>> Maybe we could also interleave the pxor with the xorps, since they
>> uses different parts of the cpu, Honza?
>>
>
>
> You almost certainly should. The reason I suggested FXRSTOR is that
> it would initialize the entire FPU, including any state that future
> processors may add, thus reducing the likelihood of any funnies in
> the future.

Here's a patch to do just that. It initializes the saved state in the
task struct and falls through to restore_fpu().

--

Brian Gerst

[-- Attachment #2: fpuclear-1 --]
[-- Type: text/plain, Size: 1951 bytes --]

diff -urN linux-2.5.8/arch/i386/kernel/i387.c linux/arch/i386/kernel/i387.c
--- linux-2.5.8/arch/i386/kernel/i387.c Thu Mar 7 21:18:32 2002
+++ linux/arch/i386/kernel/i387.c Fri Apr 19 18:23:53 2002
@@ -31,13 +31,20 @@
* value at reset if we support XMM instructions and then
* remeber the current task has used the FPU.
*/
-void init_fpu(void)
+void init_fpu(struct task_struct *tsk)
{
- __asm__("fninit");
- if ( cpu_has_xmm )
- load_mxcsr(0x1f80);
-
- current->used_math = 1;
+ if ( cpu_has_xmm ) {
+ memset(&tsk->thread.i387.fxsave, 0, sizeof(struct i387_fxsave_struct));
+ tsk->thread.i387.fxsave.cwd = 0x37f;
+ tsk->thread.i387.fxsave.mxcsr = 0x1f80;
+ } else {
+ memset(&tsk->thread.i387.fsave, 0, sizeof(struct i387_fsave_struct));
+ tsk->thread.i387.fsave.cwd = 0xffff037f;
+ tsk->thread.i387.fsave.swd = 0xffff0000;
+ tsk->thread.i387.fsave.twd = 0xffffffff;
+ tsk->thread.i387.fsave.fos = 0xffff0000;
+ }
+ tsk->used_math = 1;
}

/*
diff -urN linux-2.5.8/arch/i386/kernel/traps.c linux/arch/i386/kernel/traps.c
--- linux-2.5.8/arch/i386/kernel/traps.c Sun Apr 14 23:48:18 2002
+++ linux/arch/i386/kernel/traps.c Fri Apr 19 18:22:12 2002
@@ -757,13 +757,12 @@
*/
asmlinkage void math_state_restore(struct pt_regs regs)
{
+ struct task_struct *tsk = current;
clts(); /* Allow maths ops (or we recurse) */

- if (current->used_math) {
- restore_fpu(current);
- } else {
- init_fpu();
- }
+ if (!tsk->used_math)
+ init_fpu(tsk);
+ restore_fpu(tsk);
set_thread_flag(TIF_USEDFPU); /* So we fnsave on switch_to() */
}

diff -urN linux-2.5.8/include/asm-i386/i387.h linux/include/asm-i386/i387.h
--- linux-2.5.8/include/asm-i386/i387.h Fri Apr 19 18:44:48 2002
+++ linux/include/asm-i386/i387.h Fri Apr 19 18:46:12 2002
@@ -17,7 +17,7 @@
#include <asm/sigcontext.h>
#include <asm/user.h>

-extern void init_fpu(void);
+extern void init_fpu(struct task_struct *);
/*
* FPU lazy state save handling...
*/