SSE related security hole
Subject: Re: SSE related security hole
Reply-To:
In-Reply-To: <[email protected]>; from [email protected] on Wed, Apr 17, 2002 at 04:13:03PM +0100
(NOTE: This may have already been answered by someone else, but I haven't seen it if it has, so I'm sending this through)
Quoted 21 lines
Hi,
while debugging GCC bugreport, I noticed following behaviour of simple program with no syscalls:hubicka@nikam:~$ ./a.out sum of 7 ints: 28 hubicka@nikam:~$ ./a.out sum of 7 ints: 56 Bad sum (seen with gcc -O -march=pentiumpro -msse) hubicka@nikam:~$ ./a.out sum of 7 ints: 84 Bad sum (seen with gcc -O -march=pentiumpro -msse) hubicka@nikam:~$ ./a.out sum of 7 ints: 112 Bad sum (seen with gcc -O -march=pentiumpro -msse) hubicka@nikam:~$ echohubicka@nikam:~$ ./a.out sum of 7 ints: 28
Quoted 7 lines
ie it always returns different value, moreover when something else is run in meantime (verified by loading WWW page served by same machine), the counter is reinitialized to 28.
I am attaching the source, but it needs to be compiled by cfg-branch GCC with settings -O2 -march=pentium3 -mfpmath=sse, so I've placed static binary to http://atrey.karlin.mff.cuni.cz/~hubicka/badsum.bin
Compiling the asm source with a different compiler will also make it fail.
Quoted 3 lines
The problem appears to be reproducible only on pentium3 and athlon4 systems, not pentium4 system, where it appears to work as expected. Reproduced on both 2.4.9-RH and 2.4.16 kernels.
[ program snipped ]
So there are two different issues at play here. First, the kernel uses the fninit instruction to initialize the fpu on first use. Nothing in the Intel manuals says anything about the fninit instruction clearing the mmx or sse registers, and experimentally we now know for sure that it doesn't. That means that when the first time your program was ran it left 28 in register xmm1. The next time the program was run, the fninit did nothing to clear register xmm1 so it still held 28. Now, the pxor instruction that is part of the m() function and intended to 0 out the xmm1 register is an sse2 instruction. It just so happens that it doesn't work on sse only processors such as P3 CPUs. So, when 28 was left in xmm1, then the pxor failed to 0 out xmm1, we saved 28 as the starting value for the loop and then looped through 7 additions until we had, you guessed it, 56. In fact, if you do a while :;do bad; done loop the it will increment by 28 each time it is run except when something else intervenes. Replacing the pxor instruction with xorps instead makes it work. So, that's a bug in gcc I suspect, using sse2 instructions when only called to use sse instructions. It seems odd to me that the CPU wouldn't generate an illegal instruction exception, but oh well, it evidently doesn't.
So, we really should change arch/i386/kernel/i387.c something like this:
(WARNING, totally untested and not even compile checked change follows)
i387.c renamed from i387.c.save +19 -1
@@ -33,8 +33,26 @@void init_fpu (void ) { __asm__("fninit" ); -if ( cpu_has_xmm ) +if ( cpu_has_mmx ) + asm volatile("xorq %%mm0, %%mm0; + xorq %%mm1, %%mm1; + xorq %%mm2, %%mm2; + xorq %%mm3, %%mm3; + xorq %%mm4, %%mm4; + xorq %%mm5, %%mm5; + xorq %%mm6, %%mm6; + xorq %%mm7, %%mm7"); + if ( cpu_has_xmm ) { + asm volatile("xorps %%xmm0, %%xmm0; + xorps %%xmm1, %%xmm1; + xorps %%xmm2, %%xmm2; + xorps %%xmm3, %%xmm3; + xorps %%xmm4, %%xmm4; + xorps %%xmm5, %%xmm5; + xorps %%xmm6, %%xmm6; + xorps %%xmm7, %%xmm7"); load_mxcsr(0x1f80); + } current->used_math = 1; }
The rest of the problem is a gcc bug and possibly something that Intel should make a note of on the p3 processors (that is, that the p3 will silently fail to execute some sse2 instructions without generating the expected exception).
Signature · 4 lines
Doug Ledford <[email protected]> 919-754-3700 x44233 Red Hat, Inc. 1801 Varsity Dr. Raleigh, NC 27606
i387.c line 36
Quoted 8 lines
i387.c renamed from i387.c.save +1
-1@@ -33,8 +33,26 @@void init_fpu (void ) { __asm__("fninit" ); -if ( cpu_has_xmm ) +if ( cpu_has_mmx )
Shouldn't that be cpu_has_mmx && !cpu_has_sse2 ?
-Andi
Quoted 3 lines
i387.c renamed from i387.c.save +3
-0+ asm volatile("xorq %%mm0, %%mm0; + xorq %%mm1, %%mm1; + xorq %%mm2, %%mm2;
Quoted 6 lines · Doug Ledford
i387.c renamed from i387.c.save +0
-0@@ -33,8 +33,26 @@void init_fpu (void ) { __asm__("fninit" );
i387.c line 44
Quoted 10 lines
i387.c renamed from i387.c.save +9
-1-if ( cpu_has_xmm ) +if ( cpu_has_mmx ) + asm volatile("xorq %%mm0, %%mm0; + xorq %%mm1, %%mm1; + xorq %%mm2, %%mm2; + xorq %%mm3, %%mm3; + xorq %%mm4, %%mm4; + xorq %%mm5, %%mm5; + xorq %%mm6, %%mm6; + xorq %%mm7, %%mm7");
This mean the mmx isn't really backwards compatible and that's potentially a problem for all the legacy x86 multiuser operative systems. That's an hardware design bug, not a software problem. In short running a 2.[02] kernel on a MMX capable CPU isn't secure, the same potentially applies to windows NT and other unix, no matter of SSE.
I verified with this simple proggy:
main()
{
long long x = 2;
long long z = 3;
asm volatile("movq %0, %%mm0":: "m" (x));
asm volatile("fninit");
asm volatile("movq %%mm0, %0": "=m" (z):);
printf("%d\n", z);
}it prints 2 here, while it should print zero or at least random to be backwards compatible.
SSE was a completly different issue, that is a software bug. SSE is disabled by non aware OS, and so if we enable it we also must take care of clearing it at the first math fault.
i387.c line 53
Quoted 9 lines
i387.c renamed from i387.c.save +9
-0+if ( cpu_has_xmm ) { + asm volatile("xorps %%xmm0, %%xmm0; + xorps %%xmm1, %%xmm1; + xorps %%xmm2, %%xmm2; + xorps %%xmm3, %%xmm3; + xorps %%xmm4, %%xmm4; + xorps %%xmm5, %%xmm5; + xorps %%xmm6, %%xmm6; + xorps %%xmm7, %%xmm7");
The patch has a couple of problems. xorq doesn't exists. Since there are no params you should also drop one %. Also I think we need an emms after the mmx operations to remain binary compatible with the x86 ABI.
How does this look?
arch/i386/kernel/i387.c renamed from arch/i386/kernel/i387.c.~1~ +21 -1
@@ -33,8 +33,28 @@void init_fpu (void ) { __asm__("fninit" ); -if ( cpu_has_xmm ) +if (cpu_has_mmx) { + asmvolatile ("pxor %mm0, %mm0\n\t" +"movq %mm0, %mm1\n\t" +"movq %mm0, %mm2\n\t" +"movq %mm0, %mm3\n\t" +"movq %mm0, %mm4\n\t" +"movq %mm0, %mm5\n\t" +"movq %mm0, %mm6\n\t" +"movq %mm0, %mm7\n\t" +"emms\n" ); + } +if ( cpu_has_xmm ) { + asmvolatile ("xorps %xmm0, %xmm0\n\t" +"xorps %xmm1, %xmm1\n\t" +"xorps %xmm2, %xmm2\n\t" +"xorps %xmm3, %xmm3\n\t" +"xorps %xmm4, %xmm4\n\t" +"xorps %xmm5, %xmm5\n\t" +"xorps %xmm6, %xmm6\n\t" +"xorps %xmm7, %xmm7\n" );load_mxcsr (0x1f80 ); + } current->used_math =1 ; }
Andrea
Quoted 5 lines
This mean the mmx isn't really backwards compatible and that's potentially a problem for all the legacy x86 multiuser operative systems. That's an hardware design bug, not a software problem. In short running a 2.[02] kernel on a MMX capable CPU isn't secure, the same potentially applies to windows NT and other unix, no matter of SSE.
That was my initial reaction but when I reread the documentation the Intel folks are actually saying even back in Pentium MMX days that it isnt guaranteed that the FP/MMX state are not seperate registers
Quoted 9 lines · Alan Cox
Quoted 5 lines
This mean the mmx isn't really backwards compatible and that's potentially a problem for all the legacy x86 multiuser operative systems. That's an hardware design bug, not a software problem. In short running a 2.[02] kernel on a MMX capable CPU isn't secure, the same potentially applies to windows NT and other unix, no matter of SSE.
That was my initial reaction but when I reread the documentation the Intel folks are actually saying even back in Pentium MMX days that it isnt guaranteed that the FP/MMX state are not seperate registers
In this case it would be possible to only do the explicit clear when the CPU does support sse1. For mmx only it shouldn't be needed. For sse2 also not.
-Andi
Quoted 6 lines
Quoted 2 lines
Intel folks are actually saying even back in Pentium MMX days that it isnt guaranteed that the FP/MMX state are not seperate registers
In this case it would be possible to only do the explicit clear when the CPU does support sse1. For mmx only it shouldn't be needed. For sse2 also not.
Do you have a documentation cite for that claim ?
Quoted 8 lines · Alan Cox
Quoted 6 lines
Quoted 2 lines
Intel folks are actually saying even back in Pentium MMX days that it isnt guaranteed that the FP/MMX state are not seperate registers
In this case it would be possible to only do the explicit clear when the CPU does support sse1. For mmx only it shouldn't be needed. For sse2 also not.
Do you have a documentation cite for that claim ?
Never mind. It was a bogus suggestion and fninit indeed also doesn't clear XMM on P4 and other SSE2 implementation.
-Andi
Quoted 8 lines · Alan Cox
Quoted 6 lines
Quoted 2 lines
Intel folks are actually saying even back in Pentium MMX days that it isnt guaranteed that the FP/MMX state are not seperate registers
In this case it would be possible to only do the explicit clear when the CPU does support sse1. For mmx only it shouldn't be needed. For sse2 also not.
Do you have a documentation cite for that claim ?
Actually I did some more tests:
test program
main()
{
unsigned int i[4], o[4];
i[0] = 1; i[1] = 2; i[2] = 3; i[3] = 4;
asm("movups %1,%%xmm1 ; fninit ; movups %%xmm1,%0" : "=m" (o) : "m" (i));
printf("%x %x %x %x\n",o[0],o[1],o[2],o[3]);
asm("movups %1,%%xmm1 ; movups %%xmm1,%0" : "=m" (o) : "m" (i));
printf("%x %x %x %x\n",o[0],o[1],o[2],o[3]);
}Result on a pentium4:
./xmmbffff68c 8048431 8049640 8049660bffff68c bffff68c bffff68c 8048431
So fninit seems to change something in XMM1.
and pentium 3:
bffff81c 8048431 8049640 8049660bffff81c bffff81c bffff81c 8048431
changes something different ?
If even Intel cannot agree on this it is probably safest to do an explicit zeroing like Andrea's patch does. I retract the origina suggestion.
-Andi
Hi!
Quoted 6 lines
Quoted 2 lines
It introduces security hole: Unrelated tasks now have your top secret value you stored in one of your registers.
Well, that's been my point all along and why I sent the patch. I was not asking why leaving the registers alone instead of 0ing them out was not a security hole. I was asking why doing so was not backward compatible?
Introducing security hole counts as "poor backcompatibility" to me.
Pavel
Signature · 2 lines
(about SSSCA) "I don't say this lightly. However, I really think that the U.S. no longer is classifiable as a democracy, but rather as a plutocracy." --hpa
Quoted 84 lines · Andrea Arcangeli
Quoted 6 lines · Doug Ledford
i387.c Wed Apr 17 19:28:27 2002 renamed from i387.c.save Wed Apr 17 19:22:47 2002 +0
-0@@ -33,8 +33,26 @@ void init_fpu(void) { __asm__("fninit");i387.c Wed Apr 17 19:28:27 2002 line 44
Quoted 10 lines
i387.c Wed Apr 17 19:28:27 2002 renamed from i387.c.save Wed Apr 17 19:22:47 2002 +9
-1- if ( cpu_has_xmm ) + if ( cpu_has_mmx ) + asm volatile("xorq %%mm0, %%mm0; + xorq %%mm1, %%mm1; + xorq %%mm2, %%mm2; + xorq %%mm3, %%mm3; + xorq %%mm4, %%mm4; + xorq %%mm5, %%mm5; + xorq %%mm6, %%mm6; + xorq %%mm7, %%mm7");This mean the mmx isn't really backwards compatible and that's potentially a problem for all the legacy x86 multiuser operative systems. That's an hardware design bug, not a software problem. In short running a 2.[02] kernel on a MMX capable CPU isn't secure, the same potentially applies to windows NT and other unix, no matter of SSE.
I verified with this simple proggy:
main(){ long long x = 2; long long z = 3; asm volatile("movq %0, %%mm0":: "m" (x)); asm volatile("fninit"); asm volatile("movq %%mm0, %0": "=m" (z):); printf("%d\n", z); }it prints 2 here, while it should print zero or at least random to be backwards compatible.
SSE was a completly different issue, that is a software bug. SSE is disabled by non aware OS, and so if we enable it we also must take care of clearing it at the first math fault.
i387.c Wed Apr 17 19:28:27 2002 line 53
Quoted 9 lines
i387.c Wed Apr 17 19:28:27 2002 renamed from i387.c.save Wed Apr 17 19:22:47 2002 +9
-0+ if ( cpu_has_xmm ) { + asm volatile("xorps %%xmm0, %%xmm0; + xorps %%xmm1, %%xmm1; + xorps %%xmm2, %%xmm2; + xorps %%xmm3, %%xmm3; + xorps %%xmm4, %%xmm4; + xorps %%xmm5, %%xmm5; + xorps %%xmm6, %%xmm6; + xorps %%xmm7, %%xmm7");The patch has a couple of problems. xorq doesn't exists. Since there are no params you should also drop one %. Also I think we need an emms after the mmx operations to remain binary compatible with the x86 ABI.
How does this look?
arch/i386/kernel/i387.c Thu Apr 18 07:20:26 2002 renamed from arch/i386/kernel/i387.c.~1~ Thu Apr 18 05:23:12 2002 +15
-1@@ -33,8 +33,28 @@ void init_fpu(void) { __asm__("fninit"); - if ( cpu_has_xmm ) + if (cpu_has_mmx) { + asm volatile("pxor %mm0, %mm0\n\t" + "movq %mm0, %mm1\n\t" + "movq %mm0, %mm2\n\t" + "movq %mm0, %mm3\n\t" + "movq %mm0, %mm4\n\t" + "movq %mm0, %mm5\n\t" + "movq %mm0, %mm6\n\t" + "movq %mm0, %mm7\n\t" + "emms\n"); + } + if ( cpu_has_xmm ) { + asm volatile("xorps %xmm0, %xmm0\n\t" + "xorps %xmm1, %xmm1\n\t" + "xorps %xmm2, %xmm2\n\t"
arch/i386/kernel/i387.c Thu Apr 18 07:20:26 2002 line 60
Quoted 10 lines
arch/i386/kernel/i387.c Thu Apr 18 07:20:26 2002 renamed from arch/i386/kernel/i387.c.~1~ Thu Apr 18 05:23:12 2002 +6
-0+ "xorps %xmm3, %xmm3\n\t" + "xorps %xmm4, %xmm4\n\t" + "xorps %xmm5, %xmm5\n\t" + "xorps %xmm6, %xmm6\n\t" + "xorps %xmm7, %xmm7\n"); load_mxcsr(0x1f80); + } current->used_math = 1; }
Looks good; I just did the same thing ;)
Quoted 15 lines · Andrea Arcangeli
Quoted 14 lines · Doug Ledford
i387.c renamed from i387.c.save +7
-1@@ -33,8 +33,26 @@void init_fpu (void ) { __asm__("fninit" ); -if ( cpu_has_xmm ) +if ( cpu_has_mmx ) + asm volatile("xorq %%mm0, %%mm0; + xorq %%mm1, %%mm1; + xorq %%mm2, %%mm2; + xorq %%mm3, %%mm3; + xorq %%mm4, %%mm4; + xorq %%mm5, %%mm5;
Quoted 8 lines
i387.c line 44
Quoted 2 lines
i387.c renamed from i387.c.save +2
-0+ xorq %%mm6, %%mm6; + xorq %%mm7, %%mm7");This mean the mmx isn't really backwards compatible and that's potentially a problem for all the legacy x86 multiuser operative systems. That's an hardware design bug, not a software problem. In short running a 2.[02] kernel on a MMX capable CPU isn't secure, the same potentially applies to windows NT and other unix, no matter of SSE.
Why is that not backwards compatible? I've never heard of anywhere that specifies that the starting value in the mmx registers will be anything of consequence? Also, even though register space is (possibly) shared with the FP register stack, clearing out the MMX registers does not actually harm the FP register stack since the fninit already blows the stack away, which forces the application to load fp data before it can use the fpu again.
Quoted 14 lines
I verified with this simple proggy:
main(){ long long x = 2; long long z = 3; asm volatile("movq %0, %%mm0":: "m" (x)); asm volatile("fninit"); asm volatile("movq %%mm0, %0": "=m" (z):); printf("%d\n", z); }
Quoted 2 lines
it prints 2 here, while it should print zero or at least random to be backwards compatible.
I verified the same here last night with a similar test.
Quoted 3 lines
The patch has a couple of problems. xorq doesn't exists. Since there are no params you should also drop one %. Also I think we need an emms after the mmx operations to remain binary compatible with the x86 ABI.
I did mention that it was an untested and even uncompiled patch in my email after all ;-)
Quoted 1 line
How does this look?
I'm fine with this.
Quoted 34 lines
arch/i386/kernel/i387.c renamed from arch/i386/kernel/i387.c.~1~ +21
-1@@ -33,8 +33,28 @@void init_fpu (void ) { __asm__("fninit" ); -if ( cpu_has_xmm ) +if (cpu_has_mmx) { + asmvolatile ("pxor %mm0, %mm0\n\t" +"movq %mm0, %mm1\n\t" +"movq %mm0, %mm2\n\t" +"movq %mm0, %mm3\n\t" +"movq %mm0, %mm4\n\t" +"movq %mm0, %mm5\n\t" +"movq %mm0, %mm6\n\t" +"movq %mm0, %mm7\n\t" +"emms\n" ); + } +if ( cpu_has_xmm ) { + asmvolatile ("xorps %xmm0, %xmm0\n\t" +"xorps %xmm1, %xmm1\n\t" +"xorps %xmm2, %xmm2\n\t" +"xorps %xmm3, %xmm3\n\t" +"xorps %xmm4, %xmm4\n\t" +"xorps %xmm5, %xmm5\n\t" +"xorps %xmm6, %xmm6\n\t" +"xorps %xmm7, %xmm7\n" );load_mxcsr (0x1f80 ); + } current->used_math =1 ; }Andrea
Signature · 4 lines
Doug Ledford <[email protected]> 919-754-3700 x44233 Red Hat, Inc. 1801 Varsity Dr. Raleigh, NC 27606
Hi!
Quoted 15 lines
Quoted 6 lines
Quoted 6 lines
i387.c renamed from i387.c.save +6
-0+ asm volatile("xorq %%mm0, %%mm0; + xorq %%mm1, %%mm1; + xorq %%mm2, %%mm2; + xorq %%mm3, %%mm3; + xorq %%mm4, %%mm4; + xorq %%mm5, %%mm5;Quoted 8 lines
i387.c line 44
Quoted 2 lines
i387.c renamed from i387.c.save +2
-0+ xorq %%mm6, %%mm6; + xorq %%mm7, %%mm7");This mean the mmx isn't really backwards compatible and that's potentially a problem for all the legacy x86 multiuser operative systems. That's an hardware design bug, not a software problem. In short running a 2.[02] kernel on a MMX capable CPU isn't secure, the same potentially applies to windows NT and other unix, no matter of SSE.
Quoted 7 lines
Why is that not backwards compatible? I've never heard of anywhere that specifies that the starting value in the mmx registers will be anything of consequence? Also, even though register space is (possibly) shared with the FP register stack, clearing out the MMX registers does not actually harm the FP register stack since the fninit already blows the stack away, which forces the application to load fp data before it can use the fpu again.
It introduces security hole: Unrelated tasks now have your top secret value you stored in one of your registers.
Pavel
Signature · 2 lines
Casualities in World Trade Center: ~3k dead inside the building, cryptography in U.S.A. and free speech in Czech Republic.
Quoted 2 lines · Pavel Machek
It introduces security hole: Unrelated tasks now have your top secret value you stored in one of your registers.
Well, that's been my point all along and why I sent the patch. I was not asking why leaving the registers alone instead of 0ing them out was not a security hole. I was asking why doing so was not backward compatible?
Signature · 4 lines
Doug Ledford <[email protected]> 919-754-3700 x44233 Red Hat, Inc. 1801 Varsity Dr. Raleigh, NC 27606
Quoted 4 lines · Doug Ledford
pxor instruction with xorps instead makes it work. So, that's a bug in gcc I suspect, using sse2 instructions when only called to use sse instructions. It seems odd to me that the CPU wouldn't generate an illegal instruction exception, but oh well, it evidently doesn't.
Remember ye goode olde 6502/6510 processors used in the famous Commodore 64 computers? These don't bail out when using undefined opcodes either, some opcodes actually had undocumented but consistent behaviour and were used in "my program is shorter than yours" 1000 byte demo contests and the like.