linux-kernel - Re: [PATCH] x86/fpu: always restore_xinit_state() when !use_eager

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]

Date:	Mon, 27 Apr 2015 16:49:31 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Bobby Powers <bobbypowers@...il.com>
Cc:	linux-kernel@...r.kernel.org, x86@...nel.org,
	Borislav Petkov <bp@...e.de>, Ingo Molnar <mingo@...nel.org>,
	Andy Lutomirski <luto@...capital.net>,
	Dave Hansen <dave.hansen@...el.com>,
	Fenghua Yu <fenghua.yu@...el.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Pekka Riikonen <priikone@....fi>,
	Quentin Casasnovas <quentin.casasnovas@...cle.com>,
	Rik van Riel <riel@...hat.com>,
	Suresh Siddha <sbsiddha@...il.com>
Subject: Re: [PATCH] x86/fpu: always restore_xinit_state() when
	!use_eager_cpu()

On 04/26, Bobby Powers wrote:
>
> Oleg's commit f893959b ("x86/fpu: Don't abuse drop_init_fpu() in
> flush_thread()") removed drop_init_fpu() usage from flush_thread.
> This seems to break things for me - the Go 1.4 test suite fails all
> over the place with floating point comparision errors (offending
> commit found through bisection).

Ah, sorry. And thanks a lot!

I simply can't understand what I was thinking about. Of course, we
need to reset FPU state unconditionally, exec should start with the
new FPU state.

ACK, but could you please fix the subject and update the changelog?

The subject says "when !use_eager_cpu()", but we need to do this
if use_eager_cpu() == T.

> Note: used_math() looks at current, and should be switch to
> tsk_used_math(tsk), but even with this I see test suite breakage.

Agreed, tsk_used_math(tsk) looks more consistent even if used_math()
is technically correct. You can change this as well.

Thanks!

>  arch/x86/kernel/process.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 8213da6..c820baf5 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -156,11 +156,13 @@ void flush_thread(void)
>  		/* FPU state will be reallocated lazily at the first use. */
>  		drop_fpu(tsk);
>  		free_thread_xstate(tsk);
> -	} else if (!used_math()) {
> -		/* kthread execs. TODO: cleanup this horror. */
> -		if (WARN_ON(init_fpu(tsk)))
> -			force_sig(SIGKILL, tsk);
> -		user_fpu_begin();
> +	} else {
> +		if (!used_math()) {
> +			/* kthread execs. TODO: cleanup this horror. */
> +			if (WARN_ON(init_fpu(tsk)))
> +				force_sig(SIGKILL, tsk);
> +			user_fpu_begin();
> +		}
>  		restore_init_xstate();
>  	}
>  }
> -- 
> 2.3.6
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/