Re: [PATCH v4 26/48] KVM: arm64: gic: Introduce set_pending_state() to irq_op

On Fri, 2026-07-24 at 11:39 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> - [High] Incomplete state mirroring: hardware pending state
> desynchronizes from VGIC shadow state on guest MMIO/ITS writes.
> - [Medium] Userspace-reachable WARN_ON_ONCE when set_pending_state
> fails, causing potential Denial of Service.
> --
> 
> --- Patch [26]: [PATCH v4 26/48] KVM: arm64: gic: Introduce
> set_pending_state() to irq_op ---
> commit f6a8b8f38eb780090a64d051bdb9d419dcf75e26
> Author: Sascha Bischoff <[email protected]>
> 
> KVM: arm64: gic: Introduce set_pending_state() to irq_op
> 
> This patch introduces a set_pending_state() function pointer to
> irq_ops.
> This allows the VGIC to communicate pending state directly to
> hardware for
> cases where hardware manages parts of the interrupt lifecycle, like
> GICv5.
> It wires this new callback into kvm_vgic_inject_irq().
> 
> > The intent is for this to be used after the VGIC shadow
> > pending state has changed, allowing the backend to mirror the
> > updated
> > state into hardware.
> > 
> > This new function is plumbed into kvm_vgic_inject_irq(), and is
> > only
> > called if irq_ops are provided and this function pointer is
> > explicitly
> > set.
> 
> [Severity: High]
> If the intent is to mirror the state after the VGIC shadow pending
> state
> has changed, should this also be wired up for guest MMIO writes and
> ITS
> commands?
> 
> For example, when a guest writes to ISPENDR or ICPENDR, the shadow
> state is
> updated in arch/arm64/kvm/vgic/vgic-mmio.c:__set_pending() and
> __clear_pending(), but set_pending_state() is not called.

Those are GICv3 registers.

> 
> Similarly, in arch/arm64/kvm/vgic/vgic-
> its.c:vgic_its_cmd_handle_clear(),
> the ITS CLEAR command updates the LPI pending state without notifying
> the
> hardware.

GICv3. There's a good reason that this code wasn't updated.

> 
> Could this omission cause the hardware pending state to become
> desynchronized
> from the VGIC shadow state, leading to missed or spurious interrupts?

Uh, no. GICv3 != GICv5. This is a completely different path for a
different vGIC model.

> 
> > diff --git a/arch/arm64/kvm/vgic/vgic.c
> > b/arch/arm64/kvm/vgic/vgic.c
> > index 169a8664322d0..023792c28fc5c 100644
> > --- a/arch/arm64/kvm/vgic/vgic.c
> > +++ b/arch/arm64/kvm/vgic/vgic.c
> > @@ -563,6 +563,9 @@ int kvm_vgic_inject_irq(struct kvm *kvm, struct
> > kvm_vcpu *vcpu,
> >  	else
> >  		irq->pending_latch = true;
> >  
> > +	if (irq->ops && irq->ops->set_pending_state)
> > +		WARN_ON_ONCE(!irq->ops->set_pending_state(vcpu,
> > irq));
> 
> [Severity: Medium]
> Can a guest or unprivileged host-userspace trigger this warning?
> 
> Since kvm_vgic_inject_irq() is reachable via userspace injection
> (e.g., KVM_IRQ_LINE) or guest actions, if an adversary can create
> conditions that cause set_pending_state() to fail, they could trigger
> this WARN_ON_ONCE. This would violate the KVM guideline against
> user-reachable warnings.
> 
> Could this be replaced with a pr_warn_once() or handled by returning
> an error code?
> 

In general, I don't think we can hit this. It would mean that we've
made it this far and have managed to resolve a struct vgic_irq.

Thanks,
Sascha