From: David Woodhouse <[email protected]>

KVM_CHECK_EXTENSION(KVM_CAP_PLANES) on the system fd returns a
hardcoded 1. The KVM_CAP_PLANES opt-in added later in this series takes
its ceiling from exactly that query, so a VMM following the documented
negotiation concludes it may request one plane, and never enables VSM
at all. Return KVM_MAX_PLANES there instead.

While there, move the id bound check in kvm_vm_ioctl_create_plane()
under kvm->lock. Every KVM_ENABLE_CAP path that changes
kvm_arch_max_planes() takes that lock, and the limit only ever grows,
so an unlocked check could at worst spuriously reject a create racing
KVM_CAP_SPLIT_IRQCHIP. Checking under the lock just makes the bound the
one the plane is actually installed against.

Found by a local Sashiko review.

Fixes the system-fd KVM_CAP_PLANES query in "kvm: Implement
KVM_CAP_PLANES".

Fixes: https://lore.kernel.org/all/[email protected]/
Signed-off-by: David Woodhouse <[email protected]>
---
 arch/x86/kvm/svm/svm.c |  5 ++---
 virt/kvm/kvm_main.c    | 15 +++++++++++++--
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 1848136f3bb9..1c63eb3fcdc0 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5522,9 +5522,8 @@ static unsigned svm_max_planes(struct kvm *kvm)
         * 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.
+        * is fixed at creation, so the lockless read cannot race the
+        * rollback.
         */
        if (kvm->arch.vm_type == KVM_X86_SNP_VM)
                return sev_snp_max_planes(kvm);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8550303f5757..0136156df252 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5375,7 +5375,12 @@ static int kvm_vm_ioctl_check_extension_generic(struct 
kvm *kvm, long arg)
        case KVM_CAP_PLANES:
                if (kvm)
                        return kvm_arch_max_planes(kvm);
-               return 1;
+               /*
+                * The system fd reports the ceiling a VM may opt in to via
+                * KVM_ENABLE_CAP(KVM_CAP_PLANES); a VM fd reports what the
+                * VM actually has.
+                */
+               return KVM_MAX_PLANES;
        default:
                break;
        }
@@ -5587,6 +5592,13 @@ static int kvm_vm_ioctl_create_plane(struct kvm *kvm, 
unsigned id)
        struct file *file;
        int r, fd;
 
+       guard(mutex)(&kvm->lock);
+
+       /*
+        * Check the limit under kvm->lock, where KVM_ENABLE_CAP sets it,
+        * so a concurrent cap enable cannot change it between this check
+        * and the installation below.
+        */
        if (id >= kvm_arch_max_planes(kvm) ||
            WARN_ON_ONCE(id >= KVM_MAX_PLANES))
                return -EINVAL;
@@ -5595,7 +5607,6 @@ static int kvm_vm_ioctl_create_plane(struct kvm *kvm, 
unsigned id)
        if (!kvm_arch_irqchip_in_kernel(kvm))
                return -EINVAL;
 
-       guard(mutex)(&kvm->lock);
        if (kvm->planes[id])
                return -EEXIST;
 
-- 
2.50.1


Reply via email to