From: David Woodhouse <[email protected]> svm_max_planes() calls ____sev_snp_guest() without kvm->lock, from KVM_CHECK_EXTENSION(KVM_CAP_PLANES). That races __sev_guest_init(), which sets and rolls back sev->active/es_active/SNP_ACTIVE under kvm->lock, and userspace can force the rollback (ASID exhaustion, firmware init failure). It is exactly the rule kvm_lockdep_assert_sev_lock_held() documents, dodged by using the assertion-free helper. It also leaves KVM_CREATE_PLANE's pre-lock bound check with a limit that can change under it.
Key SNP-ness off kvm->arch.vm_type == KVM_X86_SNP_VM instead, which is fixed at VM creation. While there, tidy the max_planes plumbing: - floor sev_snp_max_planes() at 1, so a host CPUID reporting zero VMPLs cannot make the VM report no planes at all; - drop the sev_nr_vcpu_planes() declaration, which has no definition or caller; - drop the !CONFIG_KVM_AMD_SEV sev_snp_max_planes() stub, dead since its only caller sits under the same #ifdef (and its return type disagreed with the declaration). Fixes the unlocked SEV state check in "kvm: svm: Implement max_planes x86 operation". Reported-by: Sashiko <[email protected]> Closes: https://lore.kernel.org/kvm/[email protected]/ Fixes: https://lore.kernel.org/all/[email protected]/ Signed-off-by: David Woodhouse <[email protected]> --- arch/x86/kvm/svm/sev.c | 6 +++++- arch/x86/kvm/svm/svm.c | 11 ++++++++++- arch/x86/kvm/svm/svm.h | 2 -- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index bb61bdf9e336..fd7605e47216 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -6000,5 +6000,9 @@ bool sev_snp_blocked(enum inject_type type, struct kvm_vcpu *vcpu) int sev_snp_max_planes(struct kvm *kvm) { - return vmpl_levels; + /* + * CPUID 0x8000001f EBX[15:12] is host-provided data; a zero there + * must not report "no planes at all" (plane 0 always exists). + */ + return max(vmpl_levels, 1U); } diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 6cc070952b93..1848136f3bb9 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -5517,7 +5517,16 @@ static void svm_free_plane(struct kvm_plane *plane) static unsigned svm_max_planes(struct kvm *kvm) { #ifdef CONFIG_KVM_AMD_SEV - if (____sev_snp_guest(kvm)) + /* + * Key SNP-ness off the immutable vm_type, not the mutable SEV + * state: KVM_CHECK_EXTENSION(KVM_CAP_PLANES) reaches here with no + * lock held, while __sev_guest_init() sets and rolls back + * sev->active/es_active/SNP_ACTIVE under kvm->lock. The VM type + * is fixed at creation, so the lockless read neither races the + * rollback nor lets the limit change under KVM_CREATE_PLANE's + * pre-lock bound check. + */ + if (kvm->arch.vm_type == KVM_X86_SNP_VM) return sev_snp_max_planes(kvm); #endif diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h index 00ce3f4ef497..2b7663722be6 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -1069,7 +1069,6 @@ static inline bool sev_snp_is_rinj_active(struct kvm_vcpu *vcpu) return is_sev_snp_guest(vcpu) && (sev->vmsa_features & SVM_SEV_FEAT_RESTRICTED_INJECTION); }; -int sev_nr_vcpu_planes(struct kvm *kvm); int sev_snp_max_planes(struct kvm *kvm); #else static inline struct page *snp_safe_alloc_page_node(int node, gfp_t gfp) @@ -1104,7 +1103,6 @@ static inline bool sev_snp_inject(enum inject_type type, struct kvm_vcpu *vcpu) static inline void sev_snp_cancel_injection(struct kvm_vcpu *vcpu) {} static inline bool sev_snp_blocked(enum inject_type type, struct kvm_vcpu *vcpu) { return false; } static inline bool sev_snp_is_rinj_active(struct kvm_vcpu *vcpu) { return false; } -static inline unsigned sev_snp_max_planes(struct kvm *kvm) { return 1; } #endif /* vmenter.S */ -- 2.50.1
