[PATCH] KVM: Nullify irqfd->producer when add_producer() fails

[PATCH] KVM: Nullify irqfd->producer when add_producer() fails

[leixiang]

The x86 and powerpc add_producer() callbacks set irqfd->producer before the
fallible setup and never clear it on error.  The bypass manager doesn't
register a producer whose add_producer() failed -- producer->eventfd is
left NULL, so the later unregister early-returns and del_producer() is
never called -- so nothing ever drops the pointer.

For VFIO PCI the producer is embedded in struct vfio_pci_irq_ctx and freed
when the vector is disabled, after which a routing update dereferences the
dangling pointer via kvm_arch_update_irqfd_routing().

Nullify irqfd->producer on the error paths.

Fixes: 77e1b8332d1d ("KVM: x86: Decouple device assignment from IRQ bypass")
Fixes: c57875f5f9be ("KVM: PPC: Book3S HV: Enable IRQ bypass")
Cc: stable@vger.kernel.org
Signed-off-by: leixiang <leixiang@kylinos.cn>
---
 arch/powerpc/kvm/book3s_hv.c | 4 +++-
 arch/x86/kvm/irq.c           | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 61dbeea317f3..14919b76fb32 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -6114,9 +6114,11 @@ static int kvmppc_irq_bypass_add_producer_hv(struct irq_bypass_consumer *cons,
 	irqfd->producer = prod;

 	ret = kvmppc_set_passthru_irq(irqfd->kvm, prod->irq, irqfd->gsi);
-	if (ret)
+	if (ret) {
 		pr_info("kvmppc_set_passthru_irq (irq %d, gsi %d) fails: %d\n",
 			prod->irq, irqfd->gsi, ret);
+		irqfd->producer = NULL;
+	}

 	return ret;
 }
diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
index 8c62c6d4d5c1..cb8ac4b9b0d7 100644
--- a/arch/x86/kvm/irq.c
+++ b/arch/x86/kvm/irq.c
@@ -488,8 +488,10 @@ int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *cons,

 	if (irqfd->irq_entry.type == KVM_IRQ_ROUTING_MSI) {
 		ret = kvm_pi_update_irte(irqfd, &irqfd->irq_entry);
-		if (ret)
+		if (ret) {
 			kvm->arch.nr_possible_bypass_irqs--;
+			irqfd->producer = NULL;
+		}
 	}
 	spin_unlock_irq(&kvm->irqfds.lock);

--
2.45.0

Re: [PATCH] KVM: Nullify irqfd->producer when add_producer() fails

[sashiko-bot]

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

Pre-existing issues:
- [High] ARM64 completely omits setting `irqfd->producer = prod` in `kvm_arch_irq_bypass_add_producer()`, which breaks vLPI routing updates and renders its `kvm_arch_update_irqfd_routing()` hook dead code.
--

commit 52f3fa02a57237a69e9a84a214e25fb38e380184
Author: leixiang <leixiang@kylinos.cn>

KVM: Nullify irqfd->producer when add_producer() fails

The commit fixes an issue in x86 and powerpc where irqfd->producer is
left dangling after a failed add_producer() setup. It nullifies the
pointer in the error paths to prevent a use-after-free when the vector
is later disabled and routing updates occur.

> diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
> index 9519fec09ee6f..c5c2f778c6692 100644
> --- a/arch/x86/kvm/irq.c
> +++ b/arch/x86/kvm/irq.c
> @@ -488,8 +488,10 @@ int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *cons,
>  
>  	if (irqfd->irq_entry.type == KVM_IRQ_ROUTING_MSI) {
>  		ret = kvm_pi_update_irte(irqfd, &irqfd->irq_entry);
> -		if (ret)
> +		if (ret) {
>  			kvm->arch.nr_possible_bypass_irqs--;
> +			irqfd->producer = NULL;
> +		}
>  	}
>  	spin_unlock_irq(&kvm->irqfds.lock);

[Severity: High]
This isn't a bug introduced by this patch, but ARM64 suffers from a related
omission where it never initializes irqfd->producer.

Because irqfd->producer is unconditionally NULL on ARM64, the core KVM eventfd
logic silently skips calling the ARM64 hook kvm_arch_update_irqfd_routing(),
rendering it dead code:

arch/arm64/kvm/arm.c:kvm_arch_irq_bypass_add_producer() {
	...
	if (irq_entry->type != KVM_IRQ_ROUTING_MSI)
		return 0;

	return kvm_vgic_v4_set_forwarding(irqfd->kvm, prod->irq,
					  &irqfd->irq_entry);
}

Should irqfd->producer be initialized in kvm_arch_irq_bypass_add_producer()
for ARM64? Any future patch to fix this missing assignment on ARM64 will
immediately require the exact same error-path cleanup (irqfd->producer = NULL)
that this patch implements for x86 and PowerPC.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622075103.35164-1-leixiang@kylinos.cn?part=1

[RFC PATCH] KVM: arm64: Set irqfd->producer to enable vLPI routing updates

[leixiang]

ARM64's kvm_arch_irq_bypass_add_producer() never sets irqfd->producer,
so kvm_irq_routing_update() never calls the ARM64 routing hook.  That
hook unmaps the vLPI and falls back to software injection when an
irqfd's MSI routing changes; with it dead, the vLPI stays bound to the
old translation.

Set irqfd->producer once kvm_vgic_v4_set_forwarding() succeeds and
clear it in del_producer(), under irqfds.lock; the only reader,
kvm_irq_routing_update(), takes that lock.  set_forwarding() may sleep,
so it runs outside the lock and the pointer is published only on
success, needing no error cleanup.

Sent as RFC: the unmap-on-reroute path has been dormant.

Fixes: 2412405b3141 ("KVM: arm/arm64: register irq bypass consumer on ARM/ARM64")
Signed-off-by: leixiang <leixiang@kylinos.cn>
---
A review of the x86/powerpc producer-nullify patch pointed out that
ARM64 never sets irqfd->producer, so kvm_arch_update_irqfd_routing()
is dead.  This RFC fills that gap; set_forwarding() may sleep and so
can't run under irqfds.lock like the x86 path, hence the locking is
what I'd most like reviewed.

 arch/arm64/kvm/arm.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 50adfff75be8..6f1e4f526719 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -2935,6 +2935,7 @@ int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *cons,
 	struct kvm_kernel_irqfd *irqfd =
 		container_of(cons, struct kvm_kernel_irqfd, consumer);
 	struct kvm_kernel_irq_routing_entry *irq_entry = &irqfd->irq_entry;
+	int ret;

 	/*
 	 * The only thing we have a chance of directly-injecting is LPIs. Maybe
@@ -2943,8 +2944,16 @@ int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *cons,
 	if (irq_entry->type != KVM_IRQ_ROUTING_MSI)
 		return 0;

-	return kvm_vgic_v4_set_forwarding(irqfd->kvm, prod->irq,
-					  &irqfd->irq_entry);
+	ret = kvm_vgic_v4_set_forwarding(irqfd->kvm, prod->irq,
+					 &irqfd->irq_entry);
+	if (ret)
+		return ret;
+
+	spin_lock_irq(&irqfd->kvm->irqfds.lock);
+	irqfd->producer = prod;
+	spin_unlock_irq(&irqfd->kvm->irqfds.lock);
+
+	return 0;
 }

 void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
@@ -2957,6 +2966,10 @@ void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
 	if (irq_entry->type != KVM_IRQ_ROUTING_MSI)
 		return;

+	spin_lock_irq(&irqfd->kvm->irqfds.lock);
+	irqfd->producer = NULL;
+	spin_unlock_irq(&irqfd->kvm->irqfds.lock);
+
 	kvm_vgic_v4_unset_forwarding(irqfd->kvm, prod->irq);
 }

--
2.45.0

Re: [RFC PATCH] KVM: arm64: Set irqfd->producer to enable vLPI routing updates

[sashiko-bot]

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

New issues:
- [Critical] Early return in `kvm_arch_irq_bypass_del_producer` skips clearing `irqfd->producer`, leaving a dangling pointer that leads to a Use-After-Free.
- [High] Late assignment of `irqfd->producer` allows concurrent routing updates to silently skip hardware unmapping, leaking vLPI mappings.

Pre-existing issues:
- [High] Lockless and non-seqcount-protected access to `irqfd->irq_entry` causes torn reads of MSI routing fields.
--

commit 6730fdc5cdf162e59409e000dad54d0460124435
Author: leixiang <leixiang@kylinos.cn>

KVM: arm64: Set irqfd->producer to enable vLPI routing updates

ARM64's kvm_arch_irq_bypass_add_producer() never sets irqfd->producer,
so kvm_irq_routing_update() never calls the ARM64 routing hook. That
hook unmaps the vLPI and falls back to software injection when an
irqfd's MSI routing changes. Set irqfd->producer once
kvm_vgic_v4_set_forwarding() succeeds and clear it in del_producer().

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index d84e80b2ce546..54d4bb5d01899 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -2891,6 +2891,7 @@ int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *cons,
>  	struct kvm_kernel_irqfd *irqfd =
>  		container_of(cons, struct kvm_kernel_irqfd, consumer);
>  	struct kvm_kernel_irq_routing_entry *irq_entry = &irqfd->irq_entry;
> +	int ret;
>  
>  	/*
>  	 * The only thing we have a chance of directly-injecting is LPIs. Maybe
>  	 * one day...
>  	 */
>  	if (irq_entry->type != KVM_IRQ_ROUTING_MSI)
>  		return 0;

[Severity: High]
This isn't a bug introduced by this patch, but does this lockless access to
irqfd->irq_entry cause torn reads of MSI routing fields?

KVM_SET_GSI_ROUTING updates this struct under irqfds.lock and
irqfd->irq_entry_sc seqcount. The lockless read here can observe a torn
state (e.g., mismatched MSI devid and data), causing
vgic_its_resolve_lpi() to resolve an arbitrary, unintended LPI.

> -	return kvm_vgic_v4_set_forwarding(irqfd->kvm, prod->irq,
> -					  &irqfd->irq_entry);
> +	ret = kvm_vgic_v4_set_forwarding(irqfd->kvm, prod->irq,
> +					 &irqfd->irq_entry);
> +	if (ret)
> +		return ret;
> +
> +	spin_lock_irq(&irqfd->kvm->irqfds.lock);
> +	irqfd->producer = prod;

[Severity: High]
Does this late assignment of irqfd->producer allow concurrent routing updates
to silently skip hardware unmapping, leaking vLPI mappings?

kvm_vgic_v4_set_forwarding() maps the vLPI, but irqfd->producer is published
afterward. A concurrent routing update will see irqfd->producer == NULL and
skip unmapping the vLPI, which relies on irqfd->producer being set to invoke
kvm_arch_update_irqfd_routing().

Once add_producer() completes, it unconditionally publishes irqfd->producer.
The vLPI is now permanently bound in hardware despite KVM treating the
routing as non-MSI.

> +	spin_unlock_irq(&irqfd->kvm->irqfds.lock);
> +
> +	return 0;
>  }
>  
>  void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
> @@ -2913,6 +2922,10 @@ void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
>  	struct kvm_kernel_irqfd *irqfd =
>  		container_of(cons, struct kvm_kernel_irqfd, consumer);
>  	struct kvm_kernel_irq_routing_entry *irq_entry = &irqfd->irq_entry;
>  
>  	if (irq_entry->type != KVM_IRQ_ROUTING_MSI)
>  		return;

[Severity: Critical]
Does this early return based on the current routing type skip clearing
irqfd->producer, leaving a dangling pointer?

When an irqfd's routing is updated from MSI to non-MSI,
kvm_arch_update_irqfd_routing() unmaps the vLPI but intentionally leaves
irqfd->producer populated.

If the producer is later unregistered, kvm_arch_irq_bypass_del_producer()
hits this early return because the routing is now non-MSI. This completely
skips the newly added irqfd->producer = NULL assignment.

The KVM irqfd retains a dangling pointer to the freed producer. A subsequent
routing update will dereference the freed pointer via irqfd->producer->irq in
kvm_arch_update_irqfd_routing(), leading to a use-after-free.

> +	spin_lock_irq(&irqfd->kvm->irqfds.lock);
> +	irqfd->producer = NULL;
> +	spin_unlock_irq(&irqfd->kvm->irqfds.lock);
> +
>  	kvm_vgic_v4_unset_forwarding(irqfd->kvm, prod->irq);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623081433.21250-1-leixiang@kylinos.cn?part=1

Re: [RFC PATCH] KVM: arm64: Set irqfd->producer to enable vLPI routing updates

[Marc Zyngier]

+ Eric

On Tue, 23 Jun 2026 09:14:33 +0100,
leixiang <leixiang@kylinos.cn> wrote:
> 
> ARM64's kvm_arch_irq_bypass_add_producer() never sets irqfd->producer,
> so kvm_irq_routing_update() never calls the ARM64 routing hook.  That
> hook unmaps the vLPI and falls back to software injection when an
> irqfd's MSI routing changes; with it dead, the vLPI stays bound to the
> old translation.

/confused. Why should userspace be in control of anything? There is no
real "routing" involved (that's the guest's responsibility), and we
are really trying to hide the fact that VLPIs are used to deliver
guest-visible LPIs.

Can you explain what this is achieving?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [RFC PATCH] KVM: arm64: Set irqfd->producer to enable vLPI routing updates

[leixiang]

Hi Marc,

You're right, thanks. I chased the dead-code observation without
understanding what the forwarding path is for -- the translation is the
guest's, and reviving it would only unmap the vLPI without remapping,
which makes things worse, not better. I'll drop it.