Re: [RFC v2 1/2] x86/fpu: Fix state corruption in __fpu__restore_sig()

[Thomas Gleixner <tglx@xxxxxxxxxxxxx>]

On Mon, May 31 2021 at 12:01, Thomas Gleixner wrote:

> On Sat, May 29 2021 at 22:12, Andy Lutomirski wrote:
>>  /*
>> - * Clear the FPU state back to init state.
>> - *
>> - * Called by sys_execve(), by the signal handler code and by various
>> - * error paths.
>> + * Reset current's user FPU states to the init states.  The caller promises
>> + * that current's supervisor states (in memory or CPU regs as appropriate)
>> + * as well as the XSAVE header in memory are intact.
>>   */
>> -static void fpu__clear(struct fpu *fpu, bool user_only)
>> +void fpu__clear_user_states(struct fpu *fpu)
>>  {
>>  	WARN_ON_FPU(fpu != &current->thread.fpu);
>>  
>>  	if (!static_cpu_has(X86_FEATURE_FPU)) {
>
> This can only be safely called if XSAVES is available. So this check is
> bogus as it actually should check for !XSAVES. And if at all it should
> be:
>
>    if (WARN_ON_ONCE(!XSAVES))
>       ....
>
> This is exactly the stuff which causes subtle problems down the road.
>
> I have no idea why you are insisting on having this conditional at the
> call site. It's just an invitation for trouble because someone finds
> this function and calls it unconditionally. And he will miss the
> 'promise' part in the comment as I did.

And of course there is:

__fpu__restore_sig()

	if (!buf) {
                fpu__clear_user_states(fpu);
                return 0;
        }

and

handle_signal()

   if (!failed)
      fpu__clear_user_states(fpu);

which invoke that function unconditionally.

Thanks,

        tglx