lkml.org 
[lkml]   [2021]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [tip: x86/fpu] x86/fpu/xstate: Define new functions for clearing fpregs and xstates
From
Date
On 5/24/2021 9:34 AM, Andy Lutomirski wrote:
> On Sat, May 16, 2020 at 8:10 AM tip-bot2 for Fenghua Yu
> <tip-bot2@linutronix.de> wrote:
>>
>> The following commit has been merged into the x86/fpu branch of tip:
>>
>> Commit-ID: b860eb8dce5906b14e3a7f3c771e0b3d6ef61b94
>> Gitweb: https://git.kernel.org/tip/b860eb8dce5906b14e3a7f3c771e0b3d6ef61b94
>> Author: Fenghua Yu <fenghua.yu@intel.com>
>> AuthorDate: Tue, 12 May 2020 07:54:39 -07:00
>> Committer: Borislav Petkov <bp@suse.de>
>> CommitterDate: Wed, 13 May 2020 13:41:50 +02:00
>>
>> x86/fpu/xstate: Define new functions for clearing fpregs and xstates
>
> syzbot says this is busted. I've made no effort to identify the
> precise bug that is making syzbot complain, but:
>
>> /*
>> - * Clear FPU registers by setting them up from
>> - * the init fpstate:
>> + * Clear FPU registers by setting them up from the init fpstate.
>> + * Caller must do fpregs_[un]lock() around it.
>> */
>> -static inline void copy_init_fpstate_to_fpregs(void)
>> +static inline void copy_init_fpstate_to_fpregs(u64 features_mask)
>> {
>> - fpregs_lock();
>> -
>
>
>
>> if (use_xsave())
>> - copy_kernel_to_xregs(&init_fpstate.xsave, -1);
>> + copy_kernel_to_xregs(&init_fpstate.xsave, features_mask);
>> else if (static_cpu_has(X86_FEATURE_FXSR))
>> copy_kernel_to_fxregs(&init_fpstate.fxsave);
>> else
>> @@ -307,9 +305,6 @@ static inline void copy_init_fpstate_to_fpregs(void)
>>
>> if (boot_cpu_has(X86_FEATURE_OSPKE))
>> copy_init_pkru_to_fpregs();
>
> if (boot_cpu_has(X86_FEATURE_OSPKE) && (features_mask & PKRU)), perhaps?
>
>> -
>> - fpregs_mark_activate();
>> - fpregs_unlock();
>> }
>>
>> /*
>> @@ -318,18 +313,40 @@ static inline void copy_init_fpstate_to_fpregs(void)
>> * Called by sys_execve(), by the signal handler code and by various
>> * error paths.
>> */
>> -void fpu__clear(struct fpu *fpu)
>> +static void fpu__clear(struct fpu *fpu, bool user_only)
>> {
>> - WARN_ON_FPU(fpu != &current->thread.fpu); /* Almost certainly an anomaly */
>> + WARN_ON_FPU(fpu != &current->thread.fpu);
>>
>> - fpu__drop(fpu);
>> + if (!static_cpu_has(X86_FEATURE_FPU)) {
>> + fpu__drop(fpu);
>> + fpu__initialize(fpu);
>> + return;
>> + }
>>
>> - /*
>> - * Make sure fpstate is cleared and initialized.
>> - */
>> - fpu__initialize(fpu);
>> - if (static_cpu_has(X86_FEATURE_FPU))
>> - copy_init_fpstate_to_fpregs();
>> + fpregs_lock();
>> +
>> + if (user_only) {
>> + if (!fpregs_state_valid(fpu, smp_processor_id()) &&
>> + xfeatures_mask_supervisor())
>> + copy_kernel_to_xregs(&fpu->state.xsave,
>> + xfeatures_mask_supervisor());
>
> This looks correct to me.
>
> So I'm guessing that syzbot may have misattributed the problem. But
> we definitely need to clean up the XRSTOR #GP handling before CET
> lands.
>

From the crash dump, the system is doing syscall_exit_to_user_mode()
for __x64_sys_futex(). The futex syscall does not seem to modify
xstates, but upon returning to user mode, XRSTORS gets a GP. Can this
be some memory corruption? fpu__clear() is merely helping to clear the
mess and seems to be innocent.

I also run the syz repro on my Tiger Lake machine, and it only produces
segfaults (no Bad FPU state, etc.).

Yu-cheng

\
 
 \ /
  Last update: 2021-05-25 19:44    [W:0.147 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site