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

[sashiko-bot@kernel.org]

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