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

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 7/6/2026 10:22 AM, sashiko-bot@kernel.org wrote:
> 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()?

Yes, this is intended. Currently host perf has been granted the right to
sample the guest except exclude_guest attribute is set.

Thanks.


>
>>  	}
>>  
>>  	cur_fps = fpu->fpstate;