linux-kernel - Re: [PATCH 6/9] x86/fpu: Make sure x86_task_fpu() doesn't get called for PF

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, 10 Jun 2024 12:01:34 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: Ingo Molnar <mingo@...nel.org>
Cc: linux-kernel@...r.kernel.org, Andy Lutomirski <luto@...capital.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Dave Hansen <dave@...1.net>, Peter Zijlstra <peterz@...radead.org>,
	Borislav Petkov <bp@...en8.de>, Brian Gerst <brgerst@...il.com>,
	"H . Peter Anvin" <hpa@...or.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Uros Bizjak <ubizjak@...il.com>
Subject: Re: [PATCH 6/9] x86/fpu: Make sure x86_task_fpu() doesn't get called
 for PF_KTHREAD tasks during exit

The whole series looks good to me, and afaics 7/9 allows more
cleanups / improvements.

But let me ask a stupid question about fpu__drop(), I know nothing
about fpu asm.

fpu__drop() does

	/* Ignore delayed exceptions from user space */
	asm volatile("1: fwait\n"

and this comment predates the git history. Could someone explain
why exactly the exiting user-space thread needs fwait ?

And if it is needed, suppose that a kernel thread exits right
after kernel_fpu_end(), can this lead to the delayed exception?

And otoh, perhaps fpu__drop() can set TIF_NEED_FPU_LOAD to avoid
switch_fpu_prepare()->save_fpregs_to_fpstate() on its path to the
final schedule?

On 06/08, Ingo Molnar wrote:
>
>  void fpu__drop(struct task_struct *tsk)
>  {
> -	struct fpu *fpu = x86_task_fpu(tsk);
> +	struct fpu *fpu;
> +
> +	/* PF_KTHREAD tasks do not use the FPU context area: */
> +	if (tsk->flags & PF_KTHREAD)
> +		return;

I think it can already do

	if (tsk->flags & (PF_KTHREAD | PF_USER_WORKER))
		return;

This matches other similar checks. But I won't insist, and I
think all these checks need some cleanups anyway.

Oleg.