linux-kernel - Re: [syzbot] WARNING in ex_handler

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b2dddfbc-02cd-fcd8-6c89-ae6204e4cf6f@intel.com>
Date:   Thu, 27 May 2021 11:59:34 -0700
From:   "Yu, Yu-cheng" <yu-cheng.yu@...el.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Andy Lutomirski <luto@...nel.org>,
        syzbot <syzbot+2067e764dbcd10721e2e@...kaller.appspotmail.com>,
        bp@...en8.de, bp@...e.de, dave.hansen@...ux.intel.com,
        fenghua.yu@...el.com, hpa@...or.com, linux-kernel@...r.kernel.org,
        mingo@...hat.com, peterz@...radead.org,
        syzkaller-bugs@...glegroups.com, tony.luck@...el.com,
        x86@...nel.org
Subject: Re: [syzbot] WARNING in ex_handler_fprestore

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