Re: SSE related security hole

[Arjan van de Ven]

Andrea Arcangeli wrote:
> 
> On Wed, Apr 17, 2002 at 07:42:49PM -0400, Doug Ledford wrote:
> > --- i387.c.save       Wed Apr 17 19:22:47 2002
> > +++ i387.c    Wed Apr 17 19:28:27 2002
> > @@ -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");
> 
> 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.
> 
> > +     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?
> 
> --- 2.4.19pre7aa1/arch/i386/kernel/i387.c.~1~   Thu Apr 18 05:23:12 2002
> +++ 2.4.19pre7aa1/arch/i386/kernel/i387.c       Thu Apr 18 07:20:26 2002
> @@ -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"
> +                            "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 ;)