Re: [PATCH v3 29/40] KVM: arm64: gic-v5: Support SPI injection

[sashiko-bot@kernel.org]

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

New issues:
- [High] O(N^2) hypercalls inside a global raw spinlock on the guest exit path.
- [High] VFIO level-triggered SPIs are permanently lost because EOI notifications are gated behind `!irq_is_pending()`.
- [Medium] Guest-triggerable host dmesg spam via `nr_spis` clamping mismatch in IRS capability registers.

Pre-existing issues:
- [High] Spectre v1 gadget in `vgic_get_irq()` for GICv2/v3 SPIs due to subtraction after `array_index_nospec()`.
--

commit 4f4d623ce71f96a23431e620fddd39282b0000ad
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
Subject: KVM: arm64: gic-v5: Support SPI injection

This commit adds support for injecting and managing GICv5 SPIs in the VGIC
core. It introduces SPI lookup, irq_ops for state management, and a global
AP list for tracking SPIs during VCPU exits.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index 20ae7daf4c451..ceab71c2412bd 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> @@ -1113,6 +1113,99 @@ void vgic_v5_fold_irq_state(struct kvm_vcpu *vcpu)

[Severity: High]
Will this loop iteration cause soft lockups and VCPU starvation?

Looking at vgic_v5_fold_irq_state(), the host iterates over the global
vgic_v5_spi_ap_list_head while holding a raw spinlock and making hypercalls:

arch/arm64/kvm/vgic/vgic-v5.c:vgic_v5_fold_irq_state() {
retry:
    raw_spin_lock(&vgic_dist->vgic_v5_spi_ap_list_lock);
    list_for_each_entry(irq, &vgic_dist->vgic_v5_spi_ap_list_head, ap_list) {
        ...
        icsr = kvm_call_hyp_ret(__vgic_v5_vdrcfg, irq->intid);
        ...
        if (!irq->active && !pending && !irq_is_pending(irq)) {
            ...
            list_del(&irq->ap_list);
            ...
            raw_spin_unlock(&vgic_dist->vgic_v5_spi_ap_list_lock);
            ...
            goto retry;
        }
    }
}

If multiple SPIs are deactivated simultaneously, dropping the locks and
restarting the iteration from the beginning of the list requires repeatedly
evaluating already visited active SPIs, resulting in O(N^2) hypercalls
inside a raw spinlock.

[Severity: High]
Does this condition permanently lose VFIO level-triggered SPIs?

In the same loop in vgic_v5_fold_irq_state(), the End Of Interrupt (EOI)
notification is gated behind a check that irq_is_pending() is false:

arch/arm64/kvm/vgic/vgic-v5.c:vgic_v5_fold_irq_state() {
        ...
        if (!irq->active && !pending && !irq_is_pending(irq)) {
            ...
            kvm_notify_acked_irq(vcpu->kvm, 0, intid);
        ...
}

For level-triggered interrupts injected by VFIO via irqfd, KVM sets
irq->line_level = 1. The irq_is_pending() function checks this line_level
and returns true, which prevents the EOI notification from being sent.

Without the notification, the irqfd resampler never deasserts the interrupt
and clears line_level, leaving the interrupt permanently blocked.

>  	raw_spin_unlock(&vgic_dist->vgic_v5_spi_ap_list_lock);
>  }
>  
> +static bool vgic_v5_set_spi_pending_state(struct kvm_vcpu *vcpu,

[ ... ]

> diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> index ee7fc0c7104e6..26f9ff7700429 100644
> --- a/arch/arm64/kvm/vgic/vgic.c
> +++ b/arch/arm64/kvm/vgic/vgic.c
> @@ -86,19 +86,31 @@ static struct vgic_irq *vgic_get_lpi(struct kvm *kvm, u32 intid)
>   */
>  struct vgic_irq *vgic_get_irq(struct kvm *kvm, u32 intid)
>  {
> -	/* Non-private IRQs are not yet implemented for GICv5 */
> -	if (vgic_is_v5(kvm))
> -		return NULL;
> +	enum kvm_device_type type = kvm->arch.vgic.vgic_model;
>  
>  	/* SPIs */
> -	if (intid >= VGIC_NR_PRIVATE_IRQS &&
> -	    intid < (kvm->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS)) {
> -		intid = array_index_nospec(intid, kvm->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS);
> -		return &kvm->arch.vgic.spis[intid - VGIC_NR_PRIVATE_IRQS];
> +	if (__irq_is_spi(type, intid)) {
> +		switch (type) {
> +		case KVM_DEV_TYPE_ARM_VGIC_V5:
> +			intid = vgic_v5_get_hwirq_id(intid);
> +
> +			if (intid >= kvm->arch.vgic.nr_spis)
> +				return NULL;

[Severity: Medium]
Is it possible for a guest to spam the host kernel log by exploiting a
mismatch in the nr_spis values?

In kvm_vgic_v5_irs_init(), the local nr_spis variable is clamped up to
VGIC_V5_DEFAULT_NR_SPIS (32), but the global kvm->arch.vgic.nr_spis is left
unchanged (e.g., at 16).

This clamped value is exposed to the guest in the IRS IDR registers. If the
guest writes an ID between 16 and 31 to IRS_SPI_SELR, it is accepted because
vgic_v5_is_spi_selr_valid() checks against the clamped value.

When vgic_v5_mmio_read_irs_spi() attempts to read it, this check fails and
returns NULL, which then triggers an unratelimited kvm_err():

arch/arm64/kvm/vgic/vgic-irs-v5.c:vgic_v5_mmio_read_irs_spi() {
    ...
    if (!irq) {
        kvm_err("Guest trying to access SPI not backed by KVM\n");
    ...
}

> +
> +			intid = array_index_nospec(intid, kvm->arch.vgic.nr_spis);
> +			return &kvm->arch.vgic.spis[intid];
> +		default: {
> +			u32 max_intid = kvm->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS;
> +
> +			if (intid < max_intid) {
> +				intid = array_index_nospec(intid, max_intid);
> +				return &kvm->arch.vgic.spis[intid - VGIC_NR_PRIVATE_IRQS];

[Severity: High]
This is a pre-existing issue, but could this allow an out-of-bounds access
under speculative execution?

If an attacker can control intid and speculatively bypass the
__irq_is_spi() bounds check, array_index_nospec() bounds it against the upper
limit. However, a value like 0 subtracts VGIC_NR_PRIVATE_IRQS (32) and
creates a large unsigned underflow, which may allow accessing out of bounds
memory.

> +			}
> +		}}
>  	}
>  
>  	/* LPIs */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703154811.3355680-1-sascha.bischoff@arm.com?part=29