Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     "Yu\, Yu-cheng" <yu-cheng.yu@intel.com>,
        Andy Lutomirski <luto@kernel.org>,
        syzbot <syzbot+2067e764dbcd10721e2e@syzkaller.appspotmail.com>,
        bp@alien8.de, bp@suse.de, dave.hansen@linux.intel.com,
        fenghua.yu@intel.com, hpa@zytor.com, linux-kernel@vger.kernel.org,
        mingo@redhat.com, peterz@infradead.org,
        syzkaller-bugs@googlegroups.com, tony.luck@intel.com,
        x86@kernel.org
Subject: Re: [syzbot] WARNING in ex_handler_fprestore
In-Reply-To: <b2dddfbc-02cd-fcd8-6c89-ae6204e4cf6f@intel.com>
References: <0000000000004c453905c30f8334@google.com> <fe20f360-7c6c-f1bf-c3a5-403ae979fae6@kernel.org> <87sg29886g.ffs@nanos.tec.linutronix.de> <87bl8w86m3.ffs@nanos.tec.linutronix.de> <b2dddfbc-02cd-fcd8-6c89-ae6204e4cf6f@intel.com>
Date:   Thu, 27 May 2021 21:17:02 +0200
Message-ID: <871r9s7zrl.ffs@nanos.tec.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Thu, May 27 2021 at 11:59, Yu-cheng Yu wrote:
> 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?

Just because my reproducer wreckages xstate_header.reserved, you think
it's sufficient to maybe clear that field? May I recommend to study the
XSAVE documentation?

> Or do we want to check the user buffer first before copy it?

Before your patch the code was correct. So maybe study the SDM, the code
and the commit and carefully read this email plus the following ones
before you come up with more helpful suggestions.

<SNIP>
     80 lines of useless quotes
</SNIP>

See: https://people.kernel.org/tglx/notes-about-netiquette

Thanks,

        tglx