Re: [syzbot] WARNING in ex_handler_fprestore

[Thomas Gleixner <tglx@linutronix.de>]

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
          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;
}