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

[Oliver Upton <oupton@kernel.org>]

Hi Carlos,

Please wait a few days between spins, we were in the middle of a
conversation and I didn't have a chance to reply to you since yesterday.

On Fri, Jul 03, 2026 at 02:26:00PM +0200, Carlos López wrote:
> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> index 67d107e9a77d..577286069368 100644
> --- a/arch/arm64/kvm/vgic/vgic-its.c
> +++ b/arch/arm64/kvm/vgic/vgic-its.c
> @@ -116,7 +116,15 @@ 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.
> +		 */
> +		oldirq = __xa_store(&dist->lpi_xa, intid, irq, 0);
> +		ret = xa_err(oldirq);
> +		if (!ret && oldirq)
> +			kfree_rcu(oldirq, rcu);

We need to assert the assumption here that the evicted IRQ has a nonzero
refcount and is pending release. Otherwise I feel like possibly leaking
memory is better than potentially introducing a UAF.

Untested diff below.

Thanks,
Oliver

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 4477f870c7b3..621a83399184 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -116,18 +116,28 @@ 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));
-	}
-
-	xa_unlock_irqrestore(&dist->lpi_xa, flags);
+		oldirq = __xa_store(&dist->lpi_xa, intid, irq, 0);
+		ret = xa_err(oldirq);
+		if (ret) {
+			xa_unlock_irqrestore(&dist->lpi_xa, flags);
+			kfree(irq);
 
-	if (ret) {
-		xa_release(&dist->lpi_xa, intid);
-		kfree(irq);
+			return ERR_PTR(ret);
+		}
 
-		return ERR_PTR(ret);
+		/*
+		 * The expectation is that only LPIs which are pending release
+		 * could possibly be evicted by storing the new entry. Free the
+		 * old IRQ only if this is the case, as otherwise someone else
+		 * could still hold a pointer on the outgoing IRQ.
+		 */
+		if (oldirq && !WARN_ON_ONCE(refcount_read(&oldirq->refcount) &&
+					    oldirq->pending_release))
+			kfree_rcu(oldirq, rcu);
 	}
 
+	xa_unlock_irqrestore(&dist->lpi_xa, flags);
+
 	/*
 	 * We "cache" the configuration table entries in our struct vgic_irq's.
 	 * However we only have those structs for mapped IRQs, so we read in