linux-kernel - Re: [PATCH 1/4] x86/fpu: Clear XSTATE_BV[i] in save state whenever XFD[i]=1

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]

Message-ID: <jehglwyqu243ouotvrfy2cdml73fha23pglabopcskb3qjtsw4@b5spz5uw4suq>
Date: Tue, 6 Jan 2026 13:25:37 +0800
From: Yao Yuan <yaoyuan@...ux.alibaba.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Yao Yuan <yaoyuan0329os@...il.com>, 
	Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org, kvm@...r.kernel.org, 
	x86@...nel.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/4] x86/fpu: Clear XSTATE_BV[i] in save state whenever
 XFD[i]=1

On Mon, Jan 05, 2026 at 09:31:05AM +0800, Sean Christopherson wrote:
> On Sat, Jan 03, 2026, Yao Yuan wrote:
> > On Thu, Jan 01, 2026 at 10:05:13AM +0100, Paolo Bonzini wrote:
> > > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> > > index da233f20ae6f..166c380b0161 100644
> > > --- a/arch/x86/kernel/fpu/core.c
> > > +++ b/arch/x86/kernel/fpu/core.c
> > > @@ -319,10 +319,29 @@ EXPORT_SYMBOL_FOR_KVM(fpu_enable_guest_xfd_features);
> > >  #ifdef CONFIG_X86_64
> > >  void fpu_update_guest_xfd(struct fpu_guest *guest_fpu, u64 xfd)
> > >  {
> > > +	struct fpstate *fpstate = guest_fpu->fpstate;
> > > +
> > >  	fpregs_lock();
> > > -	guest_fpu->fpstate->xfd = xfd;
> > > -	if (guest_fpu->fpstate->in_use)
> > > -		xfd_update_state(guest_fpu->fpstate);
> > > +
> > > +	/*
> > > +	 * KVM's guest ABI is that setting XFD[i]=1 *can* immediately revert
> > > +	 * the save state to initialized.  Likewise, KVM_GET_XSAVE does the
> > > +	 * same as XSAVE and returns XSTATE_BV[i]=0 whenever XFD[i]=1.
> > > +	 *
> > > +	 * If the guest's FPU state is in hardware, just update XFD: the XSAVE
> > > +	 * in fpu_swap_kvm_fpstate will clear XSTATE_BV[i] whenever XFD[i]=1.
> > > +	 *
> > > +	 * If however the guest's FPU state is NOT resident in hardware, clear
> > > +	 * disabled components in XSTATE_BV now, or a subsequent XRSTOR will
> > > +	 * attempt to load disabled components and generate #NM _in the host_.
> > > +	 */
> >
> > Hi Sean and Paolo,
> >
> > > +	if (xfd && test_thread_flag(TIF_NEED_FPU_LOAD))
> > > +		fpstate->regs.xsave.header.xfeatures &= ~xfd;
> > > +
> > > +	fpstate->xfd = xfd;
> > > +	if (fpstate->in_use)
> > > +		xfd_update_state(fpstate);
> >
> > I see a *small* window that the Host IRQ can happen just after above
> > TIF_NEED_FPU_LOAD checking, which could set TIF_NEED_FPU_LOAD
>
> Only if the code using FPU from IRQ context is buggy.  More below.
>
> > but w/o clear the xfd from fpstate->regs.xsave.header.xfeatures.
> >
> > But there's WARN in in kernel_fpu_begin_mask():
> >
> > 	WARN_ON_FPU(!irq_fpu_usable());
> >
> > irq_fpu_usable()
> > {
> > 	...
> > 	/*
> > 	 * In hard interrupt context it's safe when soft interrupts
> > 	 * are enabled, which means the interrupt did not hit in
> > 	 * a fpregs_lock()'ed critical region.
> > 	 */
> > 	return !softirq_count();
> > }
> >
> > Looks we are relying on this to catch the above *small* window
> > yet, we're in fpregs_lock() region yet.
>
> Kernel use of FPU from (soft) IRQ context is required to check irq_fpu_usable()
> (e.g. via may_use_simd()), i.e. calling fpregs_lock() protects against the kernel
> using the FPU and thus setting TIF_NEED_FPU_LOAD.
>
> The WARN in kernel_fpu_begin_mask() is purely a sanity check to help detect and
> debug buggy users.

OK, I have same understanding w/ you, thanks.

Reviewed-by: Yuan Yao <yaoyuan@...ux.alibaba.com>