Re: [PATCH v4 27/48] KVM: arm64: gic-v5: Support SPI injection

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] `vgic_v5_set_irq_pend` dereferences `vcpu->kvm` without checking for NULL, causing a host kernel panic during SPI or LPI injection.
- [High] GICv5 SPIs silently break `KVM_IRQFD_FLAG_RESAMPLE` because KVM bypasses AP list queueing and never observes guest EOIs.
- [High] GICv5 SPI injection is broken for generic KVM callers because `vgic_get_irq` expects a GICv5-encoded INTID, but external callers pass raw INTIDs.

Pre-existing issues:
- [High] Pre-existing Spectre v1 vulnerability in `vgic_get_irq()` when handling GICv2/GICv3 SPIs. Speculative execution with `intid < VGIC_NR_PRIVATE_IRQS` bypasses bounds checks and leads to negative indexing on the `spis` array.
- [High] Invalid GICv5 PPI IDs silently alias to PPI 0 (SGI 0) in `vgic_get_vcpu_irq()` due to a missing architectural bounds check.
--

--- Patch [27]: [PATCH v4 27/48] KVM: arm64: gic-v5: Support SPI injection ---
commit 416f67edd52d537473f18e3ac4c5b905a4c03f05
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: arm64: gic-v5: Support SPI injection

GICv5 SPI lifecycle is handled by the GICv5 hardware once the pending
state has been injected. This change adds support for injecting SPIs to
the core VGIC code and GICv5 code.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index 7957b9da95e7c..04392b686c2e2 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> @@ -1064,6 +1064,47 @@ void vgic_v5_flush_ppi_state(struct kvm_vcpu *vcpu)

[ ... ]

> +static bool vgic_v5_spi_queue_irq_unlock(struct kvm *kvm,
> +					 struct vgic_irq *irq,
> +					 unsigned long flags)
> +	__releases(&irq->irq_lock)
> +{
> +	lockdep_assert_held(&irq->irq_lock);
> +
> +	raw_spin_unlock_irqrestore(&irq->irq_lock, flags);
> +	return true;
> +}

[Severity: High]
Does skipping the AP list queueing here break KVM_IRQFD_FLAG_RESAMPLE?

Since this function overrides the default queueing logic and prevents the SPI
from being added to the vCPU's AP list, the lifecycle becomes fully hardware
managed.

Because KVM won't fold the SPI state back, it seems we'll never observe guest
EOIs, meaning kvm_notify_acked_irq() won't be called to trigger the resample
eventfd.

Could this lead to silently hanging devices if userspace (like QEMU virtio-mmio
or VFIO) attaches an eventfd using the resample flag?

[ ... ]

> +/* Set the pending state for GICv5 SPIs and LPIs */
> +void vgic_v5_set_irq_pend(struct kvm_vcpu *vcpu, struct vgic_irq *irq)
> +{
> +	if (WARN_ON(__irq_is_ppi(KVM_DEV_TYPE_ARM_VGIC_V5, irq->intid)))
> +		return;
> +
> +	kvm_call_hyp(__vgic_v5_vdpend, irq->intid, irq_is_pending(irq),
> +		     vcpu->kvm->arch.vgic.gicv5_vm.vm_id);
> +}

[Severity: Critical]
Is it possible for vcpu to be NULL here?

If userspace configures the IRS and injects an SPI or unmapped LPI (e.g., via
KVM_IRQ_LINE) before any vCPUs are created, irq->target_vcpu could be NULL.

This would unconditionally dereference vcpu->kvm, potentially causing a host
kernel panic.

Should we add a NULL check for vcpu before dereferencing it?

> diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> index 023792c28fc5c..d571fafb07f32 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)) {

[Severity: High]
Does this break SPI injection for generic KVM callers on GICv5?

The __irq_is_spi() helper expects a GICv5-encoded INTID when type is
KVM_DEV_TYPE_ARM_VGIC_V5.

However, standard in-kernel device emulations (like the PMU) and userspace
injections via KVM_IRQ_LINE or irqfd pass raw integer INTIDs (e.g., 32).

Without the GICv5 type bits, __irq_is_spi() will return false, causing the
injection to abort with -EINVAL.

> +		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;
> +
> +			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 does this speculative bounds checking logic
have a Spectre v1 vulnerability?

The array_index_nospec() clamp is applied to intid before subtracting
VGIC_NR_PRIVATE_IRQS (32).

If intid is speculatively less than 32, the check (intid < max_intid) will
speculatively evaluate to true, and array_index_nospec will preserve the
value.

Subtracting 32 then results in a negative index, allowing speculative
out-of-bounds reads on the spis array.

> +		}}
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but looking at vgic_get_vcpu_irq()
just below this function, it appears there is a missing bounds check for GICv5
PPI IDs:

struct vgic_irq *vgic_get_vcpu_irq(struct kvm_vcpu *vcpu, u32 intid)
{
    ...
    if (__irq_is_sgi(type, intid) || __irq_is_ppi(type, intid)) {
        switch (type) {
        case KVM_DEV_TYPE_ARM_VGIC_V5:
            intid = vgic_v5_get_hwirq_id(intid);
            intid = array_index_nospec(intid, VGIC_V5_NR_PRIVATE_IRQS);
            break;

Unlike the SPI path, this extracts the hardware ID and directly applies
array_index_nospec() without an explicit bounds check (e.g. checking if
intid >= VGIC_V5_NR_PRIVATE_IRQS).

Since __irq_is_ppi() only checks the GICv5 type bits and not the ID range, an
invalid out-of-bounds PPI ID will silently alias to index 0.

Can this lead to silent corruption of SGI 0 state?

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