Messages in this thread | | | Subject | Re: [syzbot] WARNING in ex_handler_fprestore | From | "Yu, Yu-cheng" <> | Date | Thu, 27 May 2021 11:59:34 -0700 |
| |
On 5/27/2021 9:49 AM, Thomas Gleixner wrote: > On Thu, May 27 2021 at 00:03, Thomas Gleixner wrote: >> I decoded it fully by now and will send out coherent info (and hopefully >> a patch) tomorrow with brain awake. Time for bed here... > > Not yet there with a patch, but I have a trivial C reproducer. See > below. > > The failure mode is: > > signal to task > sigaction_handler() > wreckage xsave.header.reserved[0] > sig_return() > restore_fpu_from_sigframe() > try: XRSTOR from user -> #GP > copy_from_user(fpstate, task->fpu.state.xsave); > validate(task->fpu.state.xsave) -> fail > fpu__clear_user_states() > reinits FPU hardware but task state is still wreckaged
In fpu__clear_user_states(), maybe can we clear xstate_header.reserved[] as well? Or do we want to check the user buffer first before copy it?
> signal_fault() > sigsegv_handler() > sig_return() > restore_fpu_from_sigframe() > try: XRSTOR from user -> success > > schedule() > xsave() > > other task runs on same CPU so fpu state is not longer clean > > wakeup() > exit_to_user() > xrstor() -> #GP because the header is still borked. > > XSAVE does not clear the header.reserved fields.... > > The original code kinda worked because fpu__clear() reinitialized the > task fpu state, but as this code is preemptible the same issue can > happen pre 5.8 as well if I'm not missing something. I'll verify that > after dinner. > > The commit in question just made it trivial to trigger because it keeps > the broken task fpu state around. > > The whole slow path is broken in disgusting ways. I have no good idea > yet how to fix that proper and in a way which can be backported easily. > > Thanks, > > tglx > --- > #define _GNU_SOURCE > > #include <errno.h> > #include <pthread.h> > #include <signal.h> > #include <stdlib.h> > #include <stdlib.h> > #include <stdio.h> > #include <string.h> > #include <unistd.h> > #include <sched.h> > > static void sig_handler(int sig, siginfo_t *info, void *__ctxp) > { > ucontext_t *uctx = __ctxp; > mcontext_t *mctx = &uctx->uc_mcontext; > unsigned long *p = (unsigned long *) mctx->fpregs; > > /* Wreckage xsave.header.reserved[0] */ > p[66] = 0xfffffff; > } > > static void sigsegv(int sig) > { > } > > int main(void) > { > struct sigaction sa; > cpu_set_t set; > > memset(&sa, 0, sizeof(sa)); > sa.sa_sigaction = sig_handler; > sa.sa_flags = SA_SIGINFO; > sigemptyset(&sa.sa_mask); > if (sigaction(SIGUSR1, &sa, 0)) > exit(1); > > signal(SIGSEGV, sigsegv); > > CPU_ZERO(&set); > CPU_SET(0, &set); > > sched_setaffinity(getpid(), sizeof(set), &set); > > kill(0, SIGUSR1); > fork(); > sleep(1); > > return 0; > } >
| |