lkml.org 
[lkml]   [2021]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [syzbot] WARNING in ex_handler_fprestore
From
Date
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;
> }
>

\
 
 \ /
  Last update: 2021-05-27 21:00    [W:0.934 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site