| 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: <20251230205641.4092235-1-seanjc@google.com>
Date: Tue, 30 Dec 2025 12:56:41 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
Yosry Ahmed <yosry.ahmed@...ux.dev>, Kevin Cheng <chengkev@...gle.com>
Subject: [PATCH] KVM: x86: Disallow setting CPUID and/or feature MSRs if L2 is active
Extend KVM's restriction on CPUID and feature MSR changes to disallow
updates while L2 is active in addition to rejecting updates after the vCPU
has run at least once. Like post-run vCPU model updates, attempting to
react to model changes while L2 is active is practically infeasible, e.g.
KVM would need to do _something_ in response to impossible situations where
userspace has a removed a feature that was consumed as parted of nested
VM-Enter.
In practice, disallowing vCPU model changes while L2 is active is largely
uninteresting, as the only way for L2 to be active without the vCPU having
run at least once is if userspace stuffed state via KVM_SET_NESTED_STATE.
And because KVM_SET_NESTED_STATE can't put the vCPU into L2 without
userspace first defining the vCPU model, e.g. to enable SVM/VMX, modifying
the vCPU model while L2 is active would require deliberately setting the
vCPU model, then loading nested state, and then changing the model. I.e.
no sane VMM should run afoul of the new restriction, and any VMM that does
encounter problems has likely been running a broken setup for a long time.
Cc: Yosry Ahmed <yosry.ahmed@...ux.dev>
Cc: Kevin Cheng <chengkev@...gle.com>
Signed-off-by: Sean Christopherson <seanjc@...gle.com>
---
arch/x86/kvm/cpuid.c | 19 +++++++++++--------
arch/x86/kvm/mmu/mmu.c | 6 +-----
arch/x86/kvm/pmu.c | 2 +-
arch/x86/kvm/x86.c | 13 +++++++------
arch/x86/kvm/x86.h | 4 ++--
5 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 88a5426674a1..f37331ad3ad8 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -534,17 +534,20 @@ static int kvm_set_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2,
BUILD_BUG_ON(sizeof(vcpu_caps) != sizeof(vcpu->arch.cpu_caps));
/*
- * KVM does not correctly handle changing guest CPUID after KVM_RUN, as
- * MAXPHYADDR, GBPAGES support, AMD reserved bit behavior, etc.. aren't
- * tracked in kvm_mmu_page_role. As a result, KVM may miss guest page
- * faults due to reusing SPs/SPTEs. In practice no sane VMM mucks with
- * the core vCPU model on the fly. It would've been better to forbid any
- * KVM_SET_CPUID{,2} calls after KVM_RUN altogether but unfortunately
- * some VMMs (e.g. QEMU) reuse vCPU fds for CPU hotplug/unplug and do
+ * KVM does not correctly handle changing guest CPUID after KVM_RUN or
+ * while L2 is active, as MAXPHYADDR, GBPAGES support, AMD reserved bit
+ * behavior, etc. aren't tracked in kvm_mmu_page_role, and L2 state
+ * can't be adjusted (without breaking L2 in some way). As a result,
+ * KVM may reuse SPs/SPTEs and/or run L2 with bad/misconfigured state.
+ *
+ * In practice, no sane VMM mucks with the core vCPU model on the fly.
+ * It would've been better to forbid any KVM_SET_CPUID{,2} calls after
+ * KVM_RUN or KVM_SET_NESTED_STATE altogether, but unfortunately some
+ * VMMs (e.g. QEMU) reuse vCPU fds for CPU hotplug/unplug and do
* KVM_SET_CPUID{,2} again. To support this legacy behavior, check
* whether the supplied CPUID data is equal to what's already set.
*/
- if (kvm_vcpu_has_run(vcpu)) {
+ if (!kvm_can_set_cpuid_and_feature_msrs(vcpu)) {
r = kvm_cpuid_check_equal(vcpu, e2, nent);
if (r)
goto err;
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 02c450686b4a..f17324546900 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6031,11 +6031,7 @@ void kvm_mmu_after_set_cpuid(struct kvm_vcpu *vcpu)
vcpu->arch.nested_mmu.cpu_role.ext.valid = 0;
kvm_mmu_reset_context(vcpu);
- /*
- * Changing guest CPUID after KVM_RUN is forbidden, see the comment in
- * kvm_arch_vcpu_ioctl().
- */
- KVM_BUG_ON(kvm_vcpu_has_run(vcpu), vcpu->kvm);
+ KVM_BUG_ON(!kvm_can_set_cpuid_and_feature_msrs(vcpu), vcpu->kvm);
}
void kvm_mmu_reset_context(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 487ad19a236e..ff20b4102173 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -853,7 +853,7 @@ void kvm_pmu_refresh(struct kvm_vcpu *vcpu)
{
struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
- if (KVM_BUG_ON(kvm_vcpu_has_run(vcpu), vcpu->kvm))
+ if (KVM_BUG_ON(!kvm_can_set_cpuid_and_feature_msrs(vcpu), vcpu->kvm))
return;
/*
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ff8812f3a129..211d8c24a4b1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2314,13 +2314,14 @@ static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
u64 val;
/*
- * Disallow writes to immutable feature MSRs after KVM_RUN. KVM does
- * not support modifying the guest vCPU model on the fly, e.g. changing
- * the nVMX capabilities while L2 is running is nonsensical. Allow
- * writes of the same value, e.g. to allow userspace to blindly stuff
- * all MSRs when emulating RESET.
+ * Reject writes to immutable feature MSRs if the vCPU model is frozen,
+ * as KVM doesn't support modifying the guest vCPU model on the fly,
+ * e.g. changing the VMX capabilities MSRs while L2 is active is
+ * nonsensical. Allow writes of the same value, e.g. so that userspace
+ * can blindly stuff all MSRs when emulating RESET.
*/
- if (kvm_vcpu_has_run(vcpu) && kvm_is_immutable_feature_msr(index) &&
+ if (!kvm_can_set_cpuid_and_feature_msrs(vcpu) &&
+ kvm_is_immutable_feature_msr(index) &&
(do_get_msr(vcpu, index, &val) || *data != val))
return -EINVAL;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index fdab0ad49098..9084e0dfa15c 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -172,9 +172,9 @@ static inline void kvm_nested_vmexit_handle_ibrs(struct kvm_vcpu *vcpu)
indirect_branch_prediction_barrier();
}
-static inline bool kvm_vcpu_has_run(struct kvm_vcpu *vcpu)
+static inline bool kvm_can_set_cpuid_and_feature_msrs(struct kvm_vcpu *vcpu)
{
- return vcpu->arch.last_vmentry_cpu != -1;
+ return vcpu->arch.last_vmentry_cpu == -1 && !is_guest_mode(vcpu);
}
static inline void kvm_set_mp_state(struct kvm_vcpu *vcpu, int mp_state)
base-commit: 9448598b22c50c8a5bb77a9103e2d49f134c9578
--
2.52.0.351.gbe84eed79e-goog
Powered by blists - more mailing lists