Kernel: pass FPU restore exception to user process

Previously, user processes could cause a kernel panic upon FPU state
restore, by passing bogus FPU state to the kernel (through e.g.
sigreturn). With this patch, the process is now sent a SIGFPE signal
instead.
This commit is contained in:
David van Moolenbroek 2012-03-03 19:25:57 +01:00
parent 4b6a98de5f
commit 0a8a2ecfb5
10 changed files with 125 additions and 13 deletions

View file

@ -322,18 +322,24 @@ PUBLIC void save_fpu(struct proc *pr)
#endif
}
PUBLIC void restore_fpu(struct proc *pr)
PUBLIC int restore_fpu(struct proc *pr)
{
int failed;
if(!proc_used_fpu(pr)) {
fninit();
pr->p_misc_flags |= MF_FPU_INITIALIZED;
} else {
if(osfxsr_feature) {
fxrstor(pr->p_fpu_state.fpu_save_area_p);
failed = fxrstor(pr->p_fpu_state.fpu_save_area_p);
} else {
frstor(pr->p_fpu_state.fpu_save_area_p);
failed = frstor(pr->p_fpu_state.fpu_save_area_p);
}
if (failed) return EINVAL;
}
return OK;
}
PUBLIC void cpu_identify(void)

View file

@ -224,6 +224,17 @@ PUBLIC void exception_handler(int is_nested, struct exception_frame * frame)
panic("Copy involving a user pointer failed unexpectedly!");
}
}
/* Pass any error resulting from restoring FPU state, as a FPU
* exception to the process.
*/
if (((void*)frame->eip >= (void*)fxrstor &&
(void *)frame->eip <= (void*)__fxrstor_end) ||
((void*)frame->eip >= (void*)frstor &&
(void *)frame->eip <= (void*)__frstor_end)) {
frame->eip = (reg_t) __frstor_failure;
return;
}
}
if(frame->vector == PAGE_FAULT_VECTOR) {

View file

@ -99,8 +99,11 @@ _PROTOTYPE( void fninit, (void));
_PROTOTYPE( void clts, (void));
_PROTOTYPE( void fxsave, (void *));
_PROTOTYPE( void fnsave, (void *));
_PROTOTYPE( void fxrstor, (void *));
_PROTOTYPE( void frstor, (void *));
_PROTOTYPE( int fxrstor, (void *));
_PROTOTYPE( int __fxrstor_end, (void *));
_PROTOTYPE( int frstor, (void *));
_PROTOTYPE( int __frstor_end, (void *));
_PROTOTYPE( int __frstor_failure, (void *));
_PROTOTYPE( unsigned short fnstsw, (void));
_PROTOTYPE( void fnstcw, (unsigned short* cw));

View file

@ -608,21 +608,29 @@ ENTRY(fnsave)
ret
/*===========================================================================*/
/* fxrstor */
/* fxrstor */
/*===========================================================================*/
ENTRY(fxrstor)
mov 4(%esp), %eax
fxrstor (%eax) /* Do not change the operand! (gas2ack) */
fxrstor (%eax)
ENTRY(__fxrstor_end)
xor %eax, %eax
ret
/*===========================================================================*/
/* frstor */
/* frstor */
/*===========================================================================*/
ENTRY(frstor)
mov 4(%esp), %eax
frstor (%eax) /* Do not change the operand! (gas2ack) */
frstor (%eax)
ENTRY(__frstor_end)
xor %eax, %eax
ret
/* Shared exception handler for both fxrstor and frstor. */
ENTRY(__frstor_failure)
mov $1, %eax
ret
/*===========================================================================*/
/* read_cr0 */

View file

@ -548,7 +548,9 @@ LABEL(copr_not_available)
push %ebp
mov $0, %ebp
call _C_LABEL(context_stop)
jmp _C_LABEL(copr_not_available_handler)
call _C_LABEL(copr_not_available_handler)
/* reached upon failure only */
jmp _C_LABEL(switch_to_user)
copr_not_available_in_kernel:
pushl $0

View file

@ -1885,7 +1885,15 @@ PUBLIC void copr_not_available_handler(void)
* restore the current process' state and let it run again, do not
* schedule!
*/
restore_fpu(p);
if (restore_fpu(p) != OK) {
/* Restoring FPU state failed. This is always the process's own
* fault. Send a signal, and schedule another process instead.
*/
*local_fpu_owner = NULL;
cause_sig(proc_nr(p), SIGFPE);
return;
}
*local_fpu_owner = p;
context_stop(proc_addr(KERNEL));
restore_user_context(p);

View file

@ -32,7 +32,7 @@ _PROTOTYPE( void cycles_accounting_init, (void) );
_PROTOTYPE( void context_stop, (struct proc * p) );
/* this is a wrapper to make calling it from assembly easier */
_PROTOTYPE( void context_stop_idle, (void) );
_PROTOTYPE( void restore_fpu, (struct proc *) );
_PROTOTYPE( int restore_fpu, (struct proc *) );
_PROTOTYPE( void save_fpu, (struct proc *) );
_PROTOTYPE( void save_local_fpu, (struct proc *) );
_PROTOTYPE( void fpu_sigcontext, (struct proc *, struct sigframe *fr, struct sigcontext *sc) );

View file

@ -10,6 +10,7 @@ OBJ= test1 test2 test3 test4 test5 test6 test7 test8 test9 \
test30 test31 test32 test34 test35 test36 test37 test38 test39 \
test40 test41 test42 test45 test47 test48 test49 \
test50 test53 test54 test55 test58 \
test62 \
t10a t11a t11b t40a t40b t40c t40d t40e t40f t60a t60b \
ROOTOBJ= test11 test33 test43 test44 test46 test56 test60 test61
@ -113,3 +114,4 @@ test60: test60.c
t60a: t60a.c
t60b: t60b.c
test61: test61.c
test62: test62.c

View file

@ -14,7 +14,7 @@ badones= # list of tests that failed
tests=" 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 \
21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 \
41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 \
61 \
61 62 \
sh1.sh sh2.sh interp.sh"
tests_no=`expr 0`

72
test/test62.c Normal file
View file

@ -0,0 +1,72 @@
/* FPU state corruption test. This used to be able to crash the kernel. */
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <signal.h>
#include <sys/wait.h>
#include <machine/fpu.h>
#define MAX_ERROR 1
#include "common.c"
double state = 2.0;
static int count;
static void use_fpu(int n)
{
state += (double) n * 0.5;
}
static void crashed(int sig)
{
exit(EXIT_SUCCESS);
}
static void handler(int sig, int code, struct sigcontext *sc)
{
memset(&sc->sc_fpu_state, count, sizeof(sc->sc_fpu_state));
}
int main(void)
{
int status;
start(62);
subtest = 0;
signal(SIGUSR1, (void (*)(int)) handler);
/* Initialize the FPU state. This state is inherited, too. */
use_fpu(-1);
for (count = 0; count <= 255; count++) {
switch (fork()) {
case -1:
e(1);
break;
case 0:
signal(SIGFPE, crashed);
/* Load bad state into the kernel. */
if (kill(getpid(), SIGUSR1)) e(2);
/* Let the kernel restore the state. */
use_fpu(count);
exit(EXIT_SUCCESS);
default:
/* We cannot tell exactly whether what happened is correct or
* not -- certainly not in a platform-independent way. However,
* if the whole system keeps running, that's good enough.
*/
(void) wait(&status);
}
}
if (state <= 1.4 || state >= 1.6) e(3);
quit();
}