Commit 73b9c1e5 authored by Fuad Tabba's avatar Fuad Tabba Committed by Marc Zyngier
Browse files

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: default avatarBen Simner <ben.simner@cl.cam.ac.uk>
Co-developed-by: default avatarWill Deacon <willdeacon@google.com>
Signed-off-by: default avatarWill Deacon <willdeacon@google.com>
Signed-off-by: default avatarFuad Tabba <tabba@google.com>
Link: https://patch.msgid.link/20260424084908.370776-6-tabba@google.com


Signed-off-by: default avatarMarc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
parent d89fdda7

arch/arm64/kvm/hyp/nvhe/pkvm.c

+25 −13
Original line number Diff line number Diff line
@@ -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 @@ int __pkvm_init_vm(struct kvm *host_kvm, unsigned long vm_hva, 
 *	     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);