| 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
| ||
|
Message-ID: <20251224001249.1041934-3-pbonzini@redhat.com>
Date: Wed, 24 Dec 2025 01:12:46 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: linux-kernel@...r.kernel.org,
kvm@...r.kernel.org
Cc: seanjc@...gle.com,
x86@...nel.org,
stable@...r.kernel.org
Subject: [PATCH 2/5] x86, fpu: separate fpstate->xfd and guest XFD
Until now, fpstate->xfd has acted as both the guest value and the value
that the host used when executing XSAVES and XRSTORS. This is wrong: the
data in the guest's FPU might not be initialized even if a bit is
set in XFD and, when that happens, XRSTORing the guest FPU will fail
with a #NM exception *on the host*.
Instead, store the value of XFD together with XFD_ERR in struct
fpu_guest; it will still be synchronized in fpu_load_guest_fpstate(), but
the XRSTOR(S) operation will be able to load any valid state of the FPU
independent of the XFD value.
Cc: stable@...r.kernel.org
Fixes: 820a6ee944e7 ("kvm: x86: Add emulation for IA32_XFD", 2022-01-14)
Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
---
arch/x86/include/asm/fpu/api.h | 6 ++----
arch/x86/include/asm/fpu/types.h | 7 +++++++
arch/x86/kernel/fpu/core.c | 19 ++++---------------
arch/x86/kernel/fpu/xstate.h | 18 ++++++++++--------
arch/x86/kvm/x86.c | 6 +++---
5 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
index 0820b2621416..ee9ba06b7dbe 100644
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -152,11 +152,9 @@ extern int fpu_swap_kvm_fpstate(struct fpu_guest *gfpu, bool enter_guest);
extern int fpu_enable_guest_xfd_features(struct fpu_guest *guest_fpu, u64 xfeatures);
#ifdef CONFIG_X86_64
-extern void fpu_update_guest_xfd(struct fpu_guest *guest_fpu, u64 xfd);
-extern void fpu_sync_guest_vmexit_xfd_state(void);
+extern void fpu_sync_guest_vmexit_xfd_state(struct fpu_guest *gfpu);
#else
-static inline void fpu_update_guest_xfd(struct fpu_guest *guest_fpu, u64 xfd) { }
-static inline void fpu_sync_guest_vmexit_xfd_state(void) { }
+static inline void fpu_sync_guest_vmexit_xfd_state(struct fpu_guest *gfpu) { }
#endif
extern void fpu_copy_guest_fpstate_to_uabi(struct fpu_guest *gfpu, void *buf,
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 93e99d2583d6..7abe231e2ffe 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -545,6 +545,13 @@ struct fpu_guest {
*/
u64 xfeatures;
+ /*
+ * @xfd: Save the guest value. Note that this is
+ * *not* fpstate->xfd, which is the value
+ * the host uses when doing XSAVE/XRSTOR.
+ */
+ u64 xfd;
+
/*
* @xfd_err: Save the guest value.
*/
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index a480fa8c65d5..ff17c96d290a 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -317,16 +317,6 @@ int fpu_enable_guest_xfd_features(struct fpu_guest *guest_fpu, u64 xfeatures)
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)
-{
- fpregs_lock();
- guest_fpu->fpstate->xfd = xfd;
- if (guest_fpu->fpstate->in_use)
- xfd_update_state(guest_fpu->fpstate);
- fpregs_unlock();
-}
-EXPORT_SYMBOL_FOR_KVM(fpu_update_guest_xfd);
-
/**
* fpu_sync_guest_vmexit_xfd_state - Synchronize XFD MSR and software state
*
@@ -339,14 +329,12 @@ EXPORT_SYMBOL_FOR_KVM(fpu_update_guest_xfd);
* Note: It can be invoked unconditionally even when write emulation is
* enabled for the price of a then pointless MSR read.
*/
-void fpu_sync_guest_vmexit_xfd_state(void)
+void fpu_sync_guest_vmexit_xfd_state(struct fpu_guest *gfpu)
{
- struct fpstate *fpstate = x86_task_fpu(current)->fpstate;
-
lockdep_assert_irqs_disabled();
if (fpu_state_size_dynamic()) {
- rdmsrq(MSR_IA32_XFD, fpstate->xfd);
- __this_cpu_write(xfd_state, fpstate->xfd);
+ rdmsrq(MSR_IA32_XFD, gfpu->xfd);
+ __this_cpu_write(xfd_state, gfpu->xfd);
}
}
EXPORT_SYMBOL_FOR_KVM(fpu_sync_guest_vmexit_xfd_state);
@@ -890,6 +878,7 @@ void fpu_load_guest_fpstate(struct fpu_guest *gfpu)
fpregs_restore_userregs();
fpregs_assert_state_consistent();
+ xfd_set_state(gfpu->xfd);
if (gfpu->xfd_err)
wrmsrq(MSR_IA32_XFD_ERR, gfpu->xfd_err);
}
diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
index 52ce19289989..c0ce05bee637 100644
--- a/arch/x86/kernel/fpu/xstate.h
+++ b/arch/x86/kernel/fpu/xstate.h
@@ -180,26 +180,28 @@ static inline void xfd_validate_state(struct fpstate *fpstate, u64 mask, bool rs
#endif
#ifdef CONFIG_X86_64
-static inline void xfd_set_state(u64 xfd)
+static inline void __xfd_set_state(u64 xfd)
{
wrmsrq(MSR_IA32_XFD, xfd);
__this_cpu_write(xfd_state, xfd);
}
+static inline void xfd_set_state(u64 xfd)
+{
+ if (__this_cpu_read(xfd_state) != xfd)
+ __xfd_set_state(xfd);
+}
+
static inline void xfd_update_state(struct fpstate *fpstate)
{
- if (fpu_state_size_dynamic()) {
- u64 xfd = fpstate->xfd;
-
- if (__this_cpu_read(xfd_state) != xfd)
- xfd_set_state(xfd);
- }
+ if (fpu_state_size_dynamic())
+ xfd_set_state(fpstate->xfd);
}
extern int __xfd_enable_feature(u64 which, struct fpu_guest *guest_fpu);
#else
static inline void xfd_set_state(u64 xfd) { }
-
+static inline void __xfd_set_state(u64 xfd) { }
static inline void xfd_update_state(struct fpstate *fpstate) { }
static inline int __xfd_enable_feature(u64 which, struct fpu_guest *guest_fpu) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 01d95192dfc5..56fd082859bc 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4261,7 +4261,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
if (data & ~kvm_guest_supported_xfd(vcpu))
return 1;
- fpu_update_guest_xfd(&vcpu->arch.guest_fpu, data);
+ vcpu->arch.guest_fpu.xfd = data;
break;
case MSR_IA32_XFD_ERR:
if (!msr_info->host_initiated &&
@@ -4617,7 +4617,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
!guest_cpu_cap_has(vcpu, X86_FEATURE_XFD))
return 1;
- msr_info->data = vcpu->arch.guest_fpu.fpstate->xfd;
+ msr_info->data = vcpu->arch.guest_fpu.xfd;
break;
case MSR_IA32_XFD_ERR:
if (!msr_info->host_initiated &&
@@ -11405,7 +11405,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
* in #NM irqoff handler).
*/
if (vcpu->arch.xfd_no_write_intercept)
- fpu_sync_guest_vmexit_xfd_state();
+ fpu_sync_guest_vmexit_xfd_state(&vcpu->arch.guest_fpu);
kvm_x86_call(handle_exit_irqoff)(vcpu);
--
2.52.0
Powered by blists - more mailing lists