Re: [PATCH v5 1/2] KVM: arm64: vgic: Fix race between LPI release and re-registration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition between deferred LPI release and concurrent LPI registration leaks memory and triggers a host warning.
--

--- Patch [1]: [PATCH v5 1/2] KVM: arm64: vgic: Fix race between LPI release and re-registration ---
commit 0e3a1226f09c3d09f5b82c5882f2fe2063ffdca4
Author: Carlos López <clopez@suse.de>
Subject: KVM: arm64: vgic: Fix race between LPI release and re-registration

This commit addresses a race condition where an LPI can be deleted or
leaked during concurrent release and re-registration. It moves the refcount
decrement inside the xarray lock for direct releases, and makes
vgic_add_lpi() responsible for freeing 0-refcount LPIs evicted from the
xarray.

> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> index 67d107e9a77d1..4685332457270 100644
> --- a/arch/arm64/kvm/vgic/vgic-its.c
> +++ b/arch/arm64/kvm/vgic/vgic-its.c

[ ... ]

> @@ -116,7 +116,20 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
>  		kfree(irq);
>  		irq = oldirq;
>  	} else {
> -		ret = xa_err(__xa_store(&dist->lpi_xa, intid, irq, 0));
> +		/*
> +		 * The entry is either empty or contains a dead LPI (refcount=0)
> +		 * from the deferred release path, pending cleanup by
> +		 * vgic_release_deleted_lpis(). Evict and free it if present.
> +		 * Make sure that the assumption that the IRQ is pending release
> +		 * holds before freeing it, as otherwise someone else could still
> +		 * hold a pointer to the evicted struct.
> +		 */
> +		oldirq = __xa_store(&dist->lpi_xa, intid, irq, 0);
> +		ret = xa_err(oldirq);
> +		if (!ret && oldirq &&
> +		    !WARN_ON_ONCE(refcount_read(&oldirq->refcount) ||
> +				  !oldirq->pending_release))

[Severity: High]
Can this race with vgic_put_irq_norelease() and leak the oldirq structure
while incorrectly triggering the WARN_ON_ONCE()?

Looking at vgic_put_irq_norelease(), the refcount is decremented to 0 before
pending_release is set to true:

vgic_put_irq_norelease()
    if (!__vgic_put_irq(kvm, irq))
        return false;
        
    // At this point refcount is 0, but pending_release is still false
    
    irq->pending_release = true;

If CPU1 executes vgic_add_lpi() during this window, wouldn't it see
oldirq->pending_release as false, trigger the WARN_ON_ONCE, and then skip
kfree_rcu(oldirq, rcu)?

When CPU0 later resumes and sets pending_release to true, the LPI is
already evicted from the xarray. Because it is no longer in the xarray,
vgic_release_deleted_lpis() would not find it, resulting in a permanent
leak of the old LPI.

> +			kfree_rcu(oldirq, rcu);
>  	}
>  
>  	xa_unlock_irqrestore(&dist->lpi_xa, flags);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709144225.3433646-2-clopez@suse.de?part=1