Re: [Patch v9 08/24] x86/fpu: Add update_fpu_state_and_flag() helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Guest FPU state leaks to host perf samples because the NMI handler incorrectly bypasses the `__task_fpstate` check when `TIF_NEED_FPU_LOAD` is 0.
--

commit a7b6e478b8260eaf627518f9d19e511212779fab
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

x86/fpu: Add update_fpu_state_and_flag() helper

This commit introduces the update_fpu_state_and_flag() helper to save user
FPU state and set TIF_NEED_FPU_LOAD. It also adds memory barriers in the
host/guest FPU state switch path to ensure fpu->__task_fpstate is observed
correctly when non-NULL.

Link: https://lore.kernel.org/all/20251204154721.GB2619703@noisy.programming.kicks-ass.net/

> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> index 584fb9913be42..fb78e0ecd5fa8 100644
> --- a/arch/x86/kernel/fpu/core.c
> +++ b/arch/x86/kernel/fpu/core.c
[ ... ]
> @@ -383,13 +396,15 @@ int fpu_swap_kvm_fpstate(struct fpu_guest *guest_fpu, bool enter_guest)
>  
>  	/* 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);

[Severity: High]
Can guest FPU state leak into host perf samples during this window?

The commit message mentions that subsequent patches will use this guarantee
in NMI context by checking TIF_NEED_FPU_LOAD before retrieving user FPU
state from the saved task FPU state.

However, during KVM guest execution and within the VM-exit path here,
TIF_NEED_FPU_LOAD is 0 because it was cleared on guest entry and is not set
by KVM.

If an NMI hits here before restore_fpregs_from_fpstate() restores the host
state, would the NMI handler bypass the fpu->__task_fpstate check because
TIF_NEED_FPU_LOAD is 0?

arch/x86/events/core.c:x86_pmu_update_user_xregs() {
	...
	if (user_mask && test_thread_flag(TIF_NEED_FPU_LOAD)) {
		perf_regs = container_of(data->regs_user.regs,
				 struct x86_perf_regs, regs);
		fpu = x86_task_fpu(current);
		/*
		 * If __task_fpstate is set, it holds the right pointer,
		 * otherwise fpstate will.
		 */
		fps = READ_ONCE(fpu->__task_fpstate);
	...
}

If this memory check is skipped, wouldn't the NMI handler fall back to
reading directly from the hardware registers, which still contain the guest
FPU state at this point in fpu_swap_kvm_fpstate()?

>  	}
>  
>  	cur_fps = fpu->fpstate;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706015439.3040804-1-dapeng1.mi@linux.intel.com?part=8