[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <10a553a5-699f-6921-705e-9afa1a8e42de@intel.com>
Date: Tue, 25 May 2021 10:44:08 -0700
From: "Yu, Yu-cheng" <yu-cheng.yu@...el.com>
To: Andy Lutomirski <luto@...nel.org>,
LKML <linux-kernel@...r.kernel.org>
Cc: linux-tip-commits@...r.kernel.org,
Fenghua Yu <fenghua.yu@...el.com>,
Borislav Petkov <bp@...e.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Tony Luck <tony.luck@...el.com>, x86 <x86@...nel.org>,
"Shankar, Ravi V" <ravi.v.shankar@...el.com>
Subject: Re: [tip: x86/fpu] x86/fpu/xstate: Define new functions for clearing
fpregs and xstates
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@...utronix.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@...el.com>
>> AuthorDate: Tue, 12 May 2020 07:54:39 -07:00
>> Committer: Borislav Petkov <bp@...e.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 != ¤t->thread.fpu); /* Almost certainly an anomaly */
>> + WARN_ON_FPU(fpu != ¤t->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
Powered by blists - more mailing lists