Re: [PATCH v3 14/40] KVM: arm64: gic-v5: Set up VMTEs and VPE doorbells

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Early returns in `vgic_v5_teardown()` on hardware command failure skip software cleanup, leaking doorbell IRQs and causing a Use-After-Free of VCPU structures.
--

commit 0e7cc2cc83975127589c6dbceb977e9f1bd837a2
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: arm64: gic-v5: Set up VMTEs and VPE doorbells

This commit introduces the initialization and teardown paths for GICv5 VM Table
Entries and VPE doorbells. It allocates a VM ID, configures a per-VM doorbell
IRQ domain, and creates handlers for managing doorbell interrupts per VCPU.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index 6d7d9e6ed9dbf..f1cc36696669d 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c

[ ... ]

> +void vgic_v5_teardown(struct kvm *kvm)
> +{
> +	struct vgic_dist *dist = &kvm->arch.vgic;
> +	struct kvm_vcpu *vcpu, *vcpu0;
> +	unsigned long i;
> +	int rc;
> +
> +	/*
> +	 * If the VM's ID isn't valid, then we either failed init very early or
> +	 * we've been called a second time. Nothing to do here in either case.
> +	 */
> +	if (kvm->arch.vgic.gicv5_vm.vm_id == VGIC_V5_VM_ID_INVAL)
> +		return;
> +
> +	if (kvm->arch.vgic.gicv5_vm.vmte_allocated) {
> +		/* Make the VM invalid  */
> +		vcpu0 = kvm_get_vcpu(kvm, 0);
> +		rc = vgic_v5_send_command(vcpu0, VMTE_MAKE_INVALID);
> +		if (rc) {
> +			kvm_err("could not make VMTE invalid\n");
> +			return;

[Severity: High]
If the hardware command fails, this early return skips the rest of the
software cleanup including vgic_v5_free_doorbells(). The same applies to the
early return below if vgic_v5_vmte_release() fails.

If this happens during VM destruction, the VCPU structures will eventually
be freed. Since the doorbell IRQs are left active and registered with the
VCPU pointers, could a subsequent hardware doorbell IRQ fire and cause
db_handler() to write to the freed VCPU pointer, leading to a
use-after-free?

Is it possible to continue tearing down the software resources (like
freeing the doorbells) even if the hardware invalidation or release fails?

> +		}
> +
> +		kvm_for_each_vcpu(i, vcpu, kvm) {
> +			if (vgic_v5_vmte_free_vpe(vcpu))
> +				kvm_err("Failed to free VPE\n");
> +		}
> +
> +		if (vgic_v5_vmte_release(kvm)) {
> +			kvm_err("Failed to release VM 0x%x\n", dist->gicv5_vm.vm_id);
> +			return;
> +		}
> +	}
> +
> +	vgic_v5_free_doorbells(kvm, atomic_read(&kvm->online_vcpus));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703154811.3355680-1-sascha.bischoff@arm.com?part=14