[PATCH v5 0/2] KVM: arm64: vgic: Fix racy LPI release and re-registration handling

[PATCH v5 0/2] KVM: arm64: vgic: Fix racy LPI release and re-registration handling

[Carlos López]

Fix a couple of potential issues that could arise from racy LPI release
and re-registration for the same INTID.

The issue fixed in patch 1 can manifest itself through either a
leaked LPI structure, or a prematurely deleted LPI. The issue fixed
in patch 2 could materialize as a spurious -ENOMEM failure when
registering an LPI.

v5:
* Make sure evicted IRQs on the registration path are pending
  release (Oliver Upton).

v4:
* Add __GFP_ACCOUNT to patch 2 (Sashiko).

v3:
* Use refcount_dec_and_lock_irqsave() instead of unconditionally
  grabbing the xarray lock in patch 1.
* Add patch 2.

v2:
* Address Sashiko's review. Fix the direct release path by decrementing
  the refcount under the xarray spinlock, preventing a UAF that would
  have been introduced in v1.

Carlos López (2):
  KVM: arm64: vgic: Fix race between LPI release and re-registration
  KVM: arm64: vgic: Mitigate potential LPI registration failure

 arch/arm64/kvm/vgic/vgic-its.c | 16 +++++++++++++++-
 arch/arm64/kvm/vgic/vgic.c     | 10 ++++++----
 2 files changed, 21 insertions(+), 5 deletions(-)


base-commit: 1ee27dacbe5dc4def481794d899d67b0d4570094
-- 
2.51.0

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

[Carlos López]

Fix a potential race between decrementing an LPI's reference count and
evicting that structure from the LPI xarray.

LPI structures are maintained in the VGIC LPI xarray (dist->lpi_xa).
When the reference count of an LPI structure drops to zero,
vgic_release_lpi_locked() removes the structure from the xarray and
frees it under the xarray lock.

However, the release of an LPI can race with a concurrent LPI
re-registration with the same INTID via vgic_add_lpi() on another CPU,
since the reference count drop and the xarray eviction are not performed
in a single atomic step. This can happen e.g. if the guest issues a
DISCARD while the LPI is still referenced from a vCPU's active-pending
list (ap_list), and the same INTID is re-mapped via MAPTI.

Particularly, vgic_release_lpi_locked() is called from two distinct
paths: direct release via vgic_put_irq(), and deferred release via
vgic_release_deleted_lpis(). During direct release, the issue can result
in deleting a newly registered LPI from the xarray:

  CPU0 (Releasing LPI)                    CPU1 (Adding new LPI)
  ====================                    =====================
  vgic_put_irq()
      __vgic_put_irq()
          refcount_dec_and_test()
                                          vgic_add_lpi()
                                              xa_lock_irqsave()
                                              old_irq = xa_load(.., intid)
                                              vgic_try_get_irq_ref(old_irq) == false
                        new IRQ inserted -->  __xa_store(.., intid, ..)
                                              xa_unlock_irqrestore()
  xa_lock_irqsave();
  vgic_release_lpi_locked()
      __xa_erase(.., irq->intid)   <-- BUG: new IRQ is erased
      kfree_rcu(old_irq)

During the deferred release path, the old IRQ can be leaked:

  CPU0 (Releasing LPI)                    CPU1 (Adding new LPI)
  ====================                    =====================
  vgic_put_irq_norelease()
      __vgic_put_irq()
          refcount_dec_and_test()
      irq->pending_release = true
                                          vgic_add_lpi()
                                              xa_lock_irqsave()
                                              old_irq = xa_load(.., intid)
                                              vgic_try_get_irq_ref(oldirq) == false
                 BUG: old IRQ overwritten --> __xa_store(.., intid, ..)
                                              xa_unlock_irqrestore()

  vgic_release_deleted_lpis()
      xa_lock_irqsave()
      xa_for_each() { .. } <-- old IRQ with pending_release = true
                               is gone, so it cannot be released

To fix the direct release path, move the reference count drop inside
the xarray lock, making sure that vgic_add_lpi() never encounters the
to-be-released LPI.

To fix the deferred release path, since the refcount drop must happen
under a raw spinlock, the same solution does not work. Instead, update
vgic_add_lpi(), so that if it evicts a non-NULL refcount=0 LPI from the
xarray, it takes on the responsibility of releasing it. If this happens,
vgic_release_deleted_lpis() will iterate the xarray normally and will
simply not find the already released structure.

Reported-by: Claude:claude-opus-4-6
Fixes: 3a08a6ca7c37 ("KVM: arm64: vgic-v3: Use bare refcount for VGIC LPIs")
Fixes: d54594accf73 ("KVM: arm64: vgic-v3: Erase LPIs from xarray outside of raw spinlocks")
Signed-off-by: Carlos López <clopez@suse.de>
---
 arch/arm64/kvm/vgic/vgic-its.c | 15 ++++++++++++++-
 arch/arm64/kvm/vgic/vgic.c     | 10 ++++++----
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 67d107e9a77d..468533245727 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))
+			kfree_rcu(oldirq, rcu);
 	}
 
 	xa_unlock_irqrestore(&dist->lpi_xa, flags);
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 5a4768d8cd4f..cc09e0c45b46 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -167,12 +167,14 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)
 		guard(spinlock_irqsave)(&dist->lpi_xa.xa_lock);
 	}
 
-	if (!__vgic_put_irq(kvm, irq))
+	if (!irq_is_lpi(kvm, irq->intid))
 		return;
 
-	xa_lock_irqsave(&dist->lpi_xa, flags);
-	vgic_release_lpi_locked(dist, irq);
-	xa_unlock_irqrestore(&dist->lpi_xa, flags);
+	if (refcount_dec_and_lock_irqsave(&irq->refcount,
+					  &dist->lpi_xa.xa_lock, &flags)) {
+		vgic_release_lpi_locked(dist, irq);
+		xa_unlock_irqrestore(&dist->lpi_xa, flags);
+	}
 }
 
 static void vgic_release_deleted_lpis(struct kvm *kvm)
-- 
2.51.0

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

[sashiko-bot]

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

[PATCH v5 2/2] KVM: arm64: vgic: Mitigate potential LPI registration failure

[Carlos López]

Mitigate a potential failure when inserting a new LPI into the VGIC LPI
xarray.

When vgic_add_lpi() is preparing to register a new LPI, it pre-allocates
an xarray entry using xa_reserve_irq(), so that it can later perform the
insertion under the xarray lock without allocating.

However, since xa_reserve_irq() is called before acquiring such lock,
there is a potential race where xa_reserve_irq() observes a populated
entry, thus not performing the allocation, and another CPU removes that
entry before the xarray lock is grabbed to perform the insertion.

  CPU0 (Adding new LPI)                      CPU1 (Releasing LPI)
  =====================                      ===================
  vgic_add_lpi()
      /* Entry populated, does not allocate */
      xa_reserve_irq(.., intid, ..)
                                            vgic_release_deleted_lpis()
                                                xa_lock_irqsave()
                                                vgic_release_lpi_locked()
                          xarray node freed --> __xa_erase(.., intid)
                                                xa_unlock_irqrestore()
      xa_lock_irqsave()
      xa_load(.., intid) == NULL
      vgic_try_get_irq_ref(NULL) == false
      __xa_store(.., intid, irq, 0) <-- xarray node was freed, gfp=0
                                        cannot allocate, returns -ENOMEM

This can happen e.g. if the guest issues a DISCARD while the LPI is
still referenced from a vCPU's active-pending list (ap_list), and the
same INTID is re-mapped via MAPTI.

Mitigate this by passing GFP_NOWAIT to __xa_store(), so that the
allocation can happen under the lock in the rare case that this
condition is hit. Add __GFP_ACCOUNT as well to match xa_reserve_irq()'s
flags.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 1d6f83f60f79 ("KVM: arm64: vgic: Store LPIs in an xarray")
Signed-off-by: Carlos López <clopez@suse.de>
---
 arch/arm64/kvm/vgic/vgic-its.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 468533245727..eda5692c178f 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -124,7 +124,8 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
 		 * 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);
+		oldirq = __xa_store(&dist->lpi_xa, intid, irq,
+				    GFP_NOWAIT | __GFP_ACCOUNT);
 		ret = xa_err(oldirq);
 		if (!ret && oldirq &&
 		    !WARN_ON_ONCE(refcount_read(&oldirq->refcount) ||
-- 
2.51.0

Re: [PATCH v5 2/2] KVM: arm64: vgic: Mitigate potential LPI registration failure

[sashiko-bot]

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

Pre-existing issues:
- [High] The error path in `vgic_add_lpi()` calls `xa_release()`, which acquires the `lpi_xa` lock without disabling interrupts, leading to a potential deadlock.
--

--- Patch [2]: [PATCH v5 2/2] KVM: arm64: vgic: Mitigate potential LPI registration failure ---
commit c5f231166837f986fca0d6f0a35dc7e8c46a61d1
Author: Carlos López <clopez@suse.de>
Subject: KVM: arm64: vgic: Mitigate potential LPI registration failure

This commit mitigates a potential allocation failure when inserting a new LPI 
into the VGIC LPI xarray. It updates vgic_add_lpi() to pass GFP_NOWAIT and 
__GFP_ACCOUNT to __xa_store(), allowing allocation to occur under the lock if 
the reserved entry was removed concurrently.

> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> --- a/arch/arm64/kvm/vgic/vgic-its.c
> +++ b/arch/arm64/kvm/vgic/vgic-its.c
> @@ -124,7 +124,8 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
>  		 * 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);
> +		oldirq = __xa_store(&dist->lpi_xa, intid, irq,
> +				    GFP_NOWAIT | __GFP_ACCOUNT);
>  		ret = xa_err(oldirq);
>  		if (!ret && oldirq &&
>  		    !WARN_ON_ONCE(refcount_read(&oldirq->refcount) ||
>  				  !oldirq->pending_release))
>  			kfree_rcu(oldirq, rcu);
>  	}
>  
>  	xa_unlock_irqrestore(&dist->lpi_xa, flags);
>  
>  	if (ret) {
>  		xa_release(&dist->lpi_xa, intid);

[Severity: High]
This is a pre-existing issue, but does calling xa_release() here introduce a
potential lock inversion deadlock with hardirqs?

The xa_release() function internally uses xa_cmpxchg(), which acquires the 
lpi_xa.xa_lock using spin_lock() without disabling local interrupts.

If a hardware interrupt fires on this CPU while the lock is held (for example,
an eventfd signal triggering irqfd_wakeup() in hardirq context for fast-path 
MSIs), it can inject an LPI and eventually call vgic_put_irq().

Because vgic_put_irq() acquires lpi_xa.xa_lock using spin_lock_irqsave(), the 
interrupt handler would spin forever waiting for the lock held by the preempted
vgic_add_lpi() thread.

Since this patch makes it more likely for the error path to be hit (if the 
GFP_NOWAIT allocation fails), should this path be updated to manually erase 
the entry while holding the xarray lock with interrupts disabled?

>  		kfree(irq);
>  
>  		return ERR_PTR(ret);

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

Re: [PATCH v5 2/2] KVM: arm64: vgic: Mitigate potential LPI registration failure

[Carlos López]

On 7/9/26 5:09 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The error path in `vgic_add_lpi()` calls `xa_release()`, which acquires the `lpi_xa` lock without disabling interrupts, leading to a potential deadlock.
> --
> 
> --- Patch [2]: [PATCH v5 2/2] KVM: arm64: vgic: Mitigate potential LPI registration failure ---
> commit c5f231166837f986fca0d6f0a35dc7e8c46a61d1
> Author: Carlos López <clopez@suse.de>
> Subject: KVM: arm64: vgic: Mitigate potential LPI registration failure
> 
> This commit mitigates a potential allocation failure when inserting a new LPI 
> into the VGIC LPI xarray. It updates vgic_add_lpi() to pass GFP_NOWAIT and 
> __GFP_ACCOUNT to __xa_store(), allowing allocation to occur under the lock if 
> the reserved entry was removed concurrently.
> 
>> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
>> --- a/arch/arm64/kvm/vgic/vgic-its.c
>> +++ b/arch/arm64/kvm/vgic/vgic-its.c
>> @@ -124,7 +124,8 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
>>  		 * 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);
>> +		oldirq = __xa_store(&dist->lpi_xa, intid, irq,
>> +				    GFP_NOWAIT | __GFP_ACCOUNT);
>>  		ret = xa_err(oldirq);
>>  		if (!ret && oldirq &&
>>  		    !WARN_ON_ONCE(refcount_read(&oldirq->refcount) ||
>>  				  !oldirq->pending_release))
>>  			kfree_rcu(oldirq, rcu);
>>  	}
>>  
>>  	xa_unlock_irqrestore(&dist->lpi_xa, flags);
>>  
>>  	if (ret) {
>>  		xa_release(&dist->lpi_xa, intid);
> 
> [Severity: High]
> This is a pre-existing issue, but does calling xa_release() here introduce a
> potential lock inversion deadlock with hardirqs?

I clearly did not read Oliver's diff carefully enough, this does not
happen with those changes.

> The xa_release() function internally uses xa_cmpxchg(), which acquires the 
> lpi_xa.xa_lock using spin_lock() without disabling local interrupts.
> 
> If a hardware interrupt fires on this CPU while the lock is held (for example,
> an eventfd signal triggering irqfd_wakeup() in hardirq context for fast-path 
> MSIs), it can inject an LPI and eventually call vgic_put_irq().
> 
> Because vgic_put_irq() acquires lpi_xa.xa_lock using spin_lock_irqsave(), the 
> interrupt handler would spin forever waiting for the lock held by the preempted
> vgic_add_lpi() thread.
> 
> Since this patch makes it more likely for the error path to be hit (if the 
> GFP_NOWAIT allocation fails), should this path be updated to manually erase 
> the entry while holding the xarray lock with interrupts disabled?
> 
>>  		kfree(irq);
>>  
>>  		return ERR_PTR(ret);
>