KVM: arm64: Fix pin leak and publication ordering in __pkvm_init_vcpu()

Two bugs exist in the vCPU initialisation path:

1. If a check fails after hyp_pin_shared_mem() succeeds, the cleanup
   path jumps to 'unlock' without calling unpin_host_vcpu() or
   unpin_host_sve_state(), permanently leaking pin references on the
   host vCPU and SVE state pages.

   Extract a register_hyp_vcpu() helper that performs the checks and
   the store. When register_hyp_vcpu() returns an error, call
   unpin_host_vcpu() and unpin_host_sve_state() inline before falling
   through to the existing 'unlock' label.

2. register_hyp_vcpu() publishes the new vCPU pointer into
   'hyp_vm->vcpus[]' with a bare store, allowing a concurrent caller
   of pkvm_load_hyp_vcpu() to observe a partially initialised vCPU
   object.

   Ensure the store uses smp_store_release() and the load uses
   smp_load_acquire(). While 'vm_table_lock' currently serialises the
   store and the load, these barriers ensure the reader sees the fully
   initialised 'hyp_vcpu' object even if there were a lockless path or
   if the lock's own ordering guarantees were insufficient for nested
   object initialization.

Fixes: 49af6ddb8e5c ("KVM: arm64: Add infrastructure to create and track pKVM instances at EL2")
Reported-by: Ben Simner <ben.simner@cl.cam.ac.uk>
Co-developed-by: Will Deacon <willdeacon@google.com>
Signed-off-by: Will Deacon <willdeacon@google.com>
Signed-off-by: Fuad Tabba <tabba@google.com>
Link: https://patch.msgid.link/20260424084908.370776-6-tabba@google.com
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org

diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 7ed96d64d611078d21c2b70fd708ab787c19b7b1..e7496eb8562897cea1f21ccb77ebcde2abaf9eba 100644 (file)
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -266,7 +266,8 @@ struct pkvm_hyp_vcpu *pkvm_load_hyp_vcpu(pkvm_handle_t handle,
        if (hyp_vm->kvm.created_vcpus <= vcpu_idx)
                goto unlock;
 
-       hyp_vcpu = hyp_vm->vcpus[vcpu_idx];
+       /* Pairs with smp_store_release() in register_hyp_vcpu(). */
+       hyp_vcpu = smp_load_acquire(&hyp_vm->vcpus[vcpu_idx]);
        if (!hyp_vcpu)
                goto unlock;
 
@@ -860,12 +861,30 @@ err_unpin_kvm:
  *          the page-aligned size of 'struct pkvm_hyp_vcpu'.
  * Return 0 on success, negative error code on failure.
  */
+static int register_hyp_vcpu(struct pkvm_hyp_vm *hyp_vm,
+                             struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+       unsigned int idx = hyp_vcpu->vcpu.vcpu_idx;
+
+       if (idx >= hyp_vm->kvm.created_vcpus)
+               return -EINVAL;
+
+       if (hyp_vm->vcpus[idx])
+               return -EINVAL;
+
+       /*
+        * Ensure the hyp_vcpu is initialised before publishing it to
+        * the vCPU-load path via 'hyp_vm->vcpus[]'.
+        */
+       smp_store_release(&hyp_vm->vcpus[idx], hyp_vcpu);
+       return 0;
+}
+
 int __pkvm_init_vcpu(pkvm_handle_t handle, struct kvm_vcpu *host_vcpu,
                     unsigned long vcpu_hva)
 {
        struct pkvm_hyp_vcpu *hyp_vcpu;
        struct pkvm_hyp_vm *hyp_vm;
-       unsigned int idx;
        int ret;
 
        hyp_vcpu = map_donated_memory(vcpu_hva, sizeof(*hyp_vcpu));
@@ -884,18 +903,11 @@ int __pkvm_init_vcpu(pkvm_handle_t handle, struct kvm_vcpu *host_vcpu,
        if (ret)
                goto unlock;
 
-       idx = hyp_vcpu->vcpu.vcpu_idx;
-       if (idx >= hyp_vm->kvm.created_vcpus) {
-               ret = -EINVAL;
-               goto unlock;
-       }
-
-       if (hyp_vm->vcpus[idx]) {
-               ret = -EINVAL;
-               goto unlock;
+       ret = register_hyp_vcpu(hyp_vm, hyp_vcpu);
+       if (ret) {
+               unpin_host_vcpu(host_vcpu);
+               unpin_host_sve_state(hyp_vcpu);
        }
-
-       hyp_vm->vcpus[idx] = hyp_vcpu;
 unlock:
        hyp_spin_unlock(&vm_table_lock);