Re: [PATCH v6 08/49] KVM: arm64: gic-v5: Add VPE doorbell domain

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Concurrent VCPU creation during vGIC initialization causes under-allocation of resources leading to out-of-bounds access.
--

commit e10bd8258f90674edeba15cbc5bdb3a9de97b8ef
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: arm64: gic-v5: Add VPE doorbell domain

This commit adds support for a per-VM VPE doorbell irq domain for GICv5.
It creates one doorbell per VPE under the GICv5 LPI domain and populates
the doorbell IRQ number in the GICv5 CPU interface state.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index e75a66b3920aa..60bb39421ad08 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
[ ... ]
> @@ -125,6 +126,137 @@ int vgic_v5_probe(const struct gic_kvm_info *info)
[ ... ]
> +static int vgic_v5_create_per_vm_domain(struct kvm *kvm)
> +{
> +	struct vgic_v5_vm *vm = &kvm->arch.vgic.gicv5_vm;
> +	int nr_vcpus = atomic_read(&kvm->online_vcpus);
> +	int id = task_pid_nr(current);
> +	int ret, db_virq = 0;
> +
> +	if (!gicv5_global_data.lpi_domain) {
> +		kvm_err("LPI domain uninitialized, can't set up KVM Doorbells\n");
> +		return -ENODEV;
> +	}
> +
> +	vm->fwnode = irq_domain_alloc_named_id_fwnode("GICv5-vpe-db", id);
> +	if (!vm->fwnode)
> +		return -ENOMEM;
> +
> +	/*
> +	 * KVM per-VM VPE DB domain; child of LPI domain; only ever handles
> +	 * doorbells. We know how many doorbells we have, and therefore we
> +	 * create a linear domain.
> +	 */
> +	vm->domain = irq_domain_create_hierarchy(gicv5_global_data.lpi_domain,
> +						 0, nr_vcpus, vm->fwnode,
> +						 &vgic_v5_irq_db_domain_ops, vm);
> +	if (!vm->domain) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	db_virq = irq_domain_alloc_irqs(vm->domain, nr_vcpus, NUMA_NO_NODE, vm);

[Severity: Critical]
This is a pre-existing issue, but does this allocation race with concurrent
VCPU creation, leading to an under-sized doorbell domain and potential
out-of-bounds access?

Looking at vgic_init() in arch/arm64/kvm/vgic/vgic-init.c, it guards against
ongoing VCPU creation with a lockless check:

    if (kvm->created_vcpus != atomic_read(&kvm->online_vcpus))
            return -EBUSY;

If Thread 1 calls KVM_CREATE_VCPU and acquires kvm->lock, Thread 2 could
concurrently call KVM_DEV_ARM_VGIC_CTRL_INIT and acquire config_lock.

If Thread 2 evaluates the lockless check in vgic_init() before Thread 1
increments created_vcpus, Thread 2 proceeds to allocate resources sized to
the current online_vcpus count here in vgic_v5_create_per_vm_domain().

Once Thread 1 finishes creating its VCPU and increments online_vcpus, later
accesses by the new VCPU might exceed the allocated doorbell array bounds.
While this problem already affected GICv4 VPE allocation, this patch
introduces a new vulnerable allocation for GICv5 doorbell IRQs.

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