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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Guest/Userspace-reachable WARN_ON() in vgic_v5_reset() allows unprivileged Denial of Service
- [High] Concurrent vCPU creation during vgic_v5_init() leads to out-of-bounds IRQ free
- [High] Doorbell LPIs are freed and can be reallocated while still in use by unresponsive hardware
--

commit 44d8f1c654ba05556ff20d22e8e4b8b74f493342
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
KVM: arm64: gic-v5: Set up VMTEs and VPE doorbells

This commit sets up GICv5 VM table entries and VPE doorbells by allocating
a VM ID and creating a per-VM VPE doorbell IRQ domain. It also provides the
initialization and teardown paths to manage this state.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index 455161e52f81b..5b599821b2da2 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
[ ... ]
> @@ -562,33 +580,122 @@ void vgic_v5_reset(struct kvm_vcpu *vcpu)
>  	 * CPUIF (but potentially fewer in the IRS).
>  	 */
>  	vcpu->arch.vgic_cpu.num_pri_bits = 5;
> +
> +	/* Make the VPE valid in the VPET */
> +	if (WARN_ON(vgic_v5_send_command(vcpu, VPE_MAKE_VALID)))
> +		return;
> +}

[Severity: High]
Can this WARN_ON be triggered by unprivileged userspace, potentially leading
to a host panic if panic_on_warn is enabled?

If userspace calls KVM_ARM_VCPU_INIT before KVM_DEV_ARM_VGIC_CTRL_INIT has
successfully completed, the reset path reaches vgic_v5_reset() and attempts
to send VPE_MAKE_VALID. Because the VGIC has not been initialized, the
doorbell IRQ is still uninitialized (0). This causes vgic_v5_send_command()
to return -ENXIO, directly hitting this warning.

The KVM subsystem guidelines suggest that userspace-reachable warnings should
be converted to normal error handling or rate-limited prints to avoid a
denial of service.

> +
> +static void vgic_v5_free_doorbells(struct kvm *kvm, unsigned int nr_dbs)
> +{
> +	struct vgic_v5_vm *vm = &kvm->arch.vgic.gicv5_vm;
> +	struct kvm_vcpu *vcpu;
> +	unsigned long i;
> +	int db;
> +
> +	for (i = 0; i < nr_dbs; i++) {
> +		vcpu = kvm_get_vcpu(kvm, i);
> +		db = vgic_v5_vpe_db(vcpu);
> +		if (!db)
> +			continue;
> +
> +		free_irq(db, vcpu);
> +		vcpu->arch.vgic_cpu.vgic_v5.gicv5_vpe.db = 0;
> +	}
> +
> +	if (vm->vpe_db_base) {
> +		irq_domain_free_irqs(vm->vpe_db_base,
> +				     atomic_read(&kvm->online_vcpus));

[Severity: High]
Does this free more descriptors than were originally allocated if a vCPU is
created concurrently?

During vgic_v5_init(), the doorbell domain is sized based on a snapshot of
atomic_read(&kvm->online_vcpus). If a userspace thread calls KVM_CREATE_VCPU
concurrently, it increments created_vcpus and blocks on the config_lock.
Initialization then proceeds with the older, smaller online_vcpus count.
Once the vCPU creation completes, online_vcpus is incremented.

When the VM is torn down, this code uses the new, larger online_vcpus count
to free the domain interrupts, which results in an out-of-bounds free that
could corrupt the LPI domain.

> +		vm->vpe_db_base = 0;
> +	}
> +}
> +
> +void vgic_v5_teardown(struct kvm *kvm)
> +{
> +	struct vgic_dist *dist = &kvm->arch.vgic;
> +	struct kvm_vcpu *vcpu, *vcpu0;
> +	bool release_vm_id = true;
> +	unsigned long i;
> +	int rc;
> +
> +	lockdep_assert_held(&kvm->arch.config_lock);
> +
> +	/*
> +	 * 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");
> +			release_vm_id = false;
> +			goto out_free_doorbells;

[Severity: High]
If the IRS hardware is wedged and VMTE_MAKE_INVALID fails, does jumping
straight to out_free_doorbells cause a use-after-free of the LPIs?

The error path correctly quarantines the VM ID by clearing release_vm_id to
prevent immediate reuse. However, unconditionally branching to
out_free_doorbells returns the doorbell IRQs to the global allocator. Since
the wedged hardware still considers the VMTE valid, it could continue firing
interrupts using the freed LPI numbers, potentially corrupting state if
those LPIs are reallocated to other devices or VMs.

> +		}
[ ... ]

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