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

[Sascha Bischoff <Sascha.Bischoff@arm.com>]

On Fri, 2026-07-03 at 16:27 +0000, sashiko-bot@kernel.org wrote:
> 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?

Right, that's actually a good point. I've reworked this to always free
the doorbells, even if other bits of the teardown fail.

I hope that we're never going to be in this situation as it implies
broken hardware, but it is always better to not leave dangling pointers
around.

> 
> > +		}
> > +
> > +		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));
> 

Thanks,
Sascha