kern/98460 : [kernel] [patch] fpu_clean_state() cannot be disabled for not AMD processors, those are not vulnerable to FreeBSD-SA-06:14.fpu

[Rostislav Krasny]

The following reply was made to PR kern/98460; it has been noted by GNATS.

From: Rostislav Krasny <rosti.bsd at gmail.com>
To: Bruce Evans <bde at zeta.org.au>
Cc: freebsd-gnats-submit at freebsd.org
Subject: Re: kern/98460 : [kernel] [patch] fpu_clean_state() cannot be
 disabled for not AMD processors, those are not vulnerable to
 FreeBSD-SA-06:14.fpu
Date: Fri, 16 Jun 2006 02:04:47 +0300

 On Sat, 10 Jun 2006 11:26:20 +1000 (EST)
 Bruce Evans <bde at zeta.org.au> wrote:
 
 > On Fri, 9 Jun 2006, Rostislav Krasny wrote:
 > 
 > > On Wed, 7 Jun 2006 12:09:10 +1000 (EST)
 > > Bruce Evans <bde at zeta.org.au> wrote:
 > >
 > >>> And then you want to call the fpu_clean_state() function conditionally,
 > >>> like in following example?
 > >>>
 > >>> if (cpu_fxsr & CPU_FXSR_NEEDCLEAN)
 > >>>        fpu_clean_state();
 > >>
 > >> Not quite like that.  In my version there is no function call -- the code
 > >> is excecuted in the one place where it is needed, so there is no function
 > >> call overhead or possible branch prediction oferhead for the function call.
 > >
 > > Could you please explain in more detail how that can be done?
 > 
 > Just do it.  The easiest way is define the new function as inline.
 > This just works because the function is defined before it is used.
 >
 > [snipped]
 
 But you still check cpu_fxsr, so a branch misprediction on a good few
 CPUs is still possible. The only solution is a self-modified code with
 a direct jump. I made following userland example of such a code:
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/mman.h>
 
 void fu(int n)
 {
 	mprotect(&&lab0, 1, PROT_READ|PROT_WRITE|PROT_EXEC);
 lab0:
 	asm volatile ("			\n\
 		.byte	0xE9;		\n\
 	l0:	.long	0x00000000	\n\
 	l1:	bt	$0,%%ax;	\n\
 		jnc	l2;		\n\
 		movl	$l4,%%eax;	\n\
 		subl	$l1,%%eax;	\n\
 		movl	%%eax,l0;	\n\
 		movl	$0,%%eax;	\n\
 		jmp	l4;		\n\
 	l2:	movl	$l3,%%eax;	\n\
 		subl	$l1,%%eax;	\n\
 		movl	%%eax,l0;	\n\
 		movl	$0,%%eax;	\n\
 		jmp	l4;		\n\
 	l3:	addl	$0x0A,%%eax;	\n\
 	l4:				\n\
 		" : "=a"(n) : "a"(n));
 	printf("%d\n", n);
 }
 
 int main(int argc, char *argv[])
 {
 	int n;
 
 	if (argc > 1)
 		n = atoi(argv[1]);
 	fu(n);
 	fu(n);
 	fu(n);
 
 	return 0;
 }
 
 For a demonstration purpose I made the fu() so that the first call of it
 will printf 0, to show when code modification is done. All further calls
 of fu() will print n or n+10, according to the first n.
 
 I think there should be no need in mprotect() in the kernel. That
 technique could be combined with an assembly version of fpu_clean_state()
 from following article. See the '"FXRSTOR-centric" method':
 
 http://security.freebsd.org/advisories/FreeBSD-SA-06:14-amd.txt
 
 That might be tricky, I know. But why one should pay a performance
 penalty because of a CPU he/she didn't buy?