linux-kernel - Re: [Patch v6 08/22] x86/fpu: Ensure TIF_NEED_FPU

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] [day] [month] [year] [list]

Message-ID: <960918ce-f42f-464f-a887-c36d8d9e2937@intel.com>
Date: Wed, 11 Feb 2026 11:55:10 -0800
From: Dave Hansen <dave.hansen@...el.com>
To: "Chang S. Bae" <chang.seok.bae@...el.com>,
 Dapeng Mi <dapeng1.mi@...ux.intel.com>, Peter Zijlstra
 <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
 Dave Hansen <dave.hansen@...ux.intel.com>, Ian Rogers <irogers@...gle.com>,
 Adrian Hunter <adrian.hunter@...el.com>, Jiri Olsa <jolsa@...nel.org>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Andi Kleen <ak@...ux.intel.com>, Eranian Stephane <eranian@...gle.com>
Cc: Mark Rutland <mark.rutland@....com>, broonie@...nel.org,
 Ravi Bangoria <ravi.bangoria@....com>, linux-kernel@...r.kernel.org,
 linux-perf-users@...r.kernel.org, Zide Chen <zide.chen@...el.com>,
 Falcon Thomas <thomas.falcon@...el.com>, Dapeng Mi <dapeng1.mi@...el.com>,
 Xudong Hao <xudong.hao@...el.com>
Subject: Re: [Patch v6 08/22] x86/fpu: Ensure TIF_NEED_FPU_LOAD is set after
 saving FPU state

On 2/11/26 11:39, Chang S. Bae wrote:
> But the changelog doesn't look clear enough to explain the reason to me.

Yeah, the changelog could use some improvement.

This also leaves the code rather fragile. Right now, all of the:

	save_fpregs_to_fpstate(fpu);
	set_thread_flag(TIF_NEED_FPU_LOAD);

pairs are open-coded. There's precisely nothing stopping someone from
coming in tomorrow and reversing the order at these or other call sites.
There's also zero comments left in the code to tell folks not to do this.

Are there enough of these to have a helper that does:

	save_fpregs_to_fpstate_before_invalidation(fpu);

which could do the save and set TIF_NEED_FPU_LOAD? (that name is awful
btw, please don't use it).

This:

>  	/* Swap fpstate */
>  	if (enter_guest) {
> -		fpu->__task_fpstate = cur_fps;
> +		WRITE_ONCE(fpu->__task_fpstate, cur_fps);
> +		barrier();
>  		fpu->fpstate = guest_fps;
>  		guest_fps->in_use = true;
>  	} else {
>  		guest_fps->in_use = false;
>  		fpu->fpstate = fpu->__task_fpstate;
> -		fpu->__task_fpstate = NULL;
> +		barrier();
> +		WRITE_ONCE(fpu->__task_fpstate, NULL);
>  	}

also urgently needs comments.

I also can't help but think that there might be a nicer way to do that
without the barrier(). I _think_ two correctly-ordered WRITE_ONCE()'s
would make the compiler do the same thing as the barrier().

But I'm not fully understanding what the barrier() is doing anyway, so
take that with a grain of salt.