Re: [PATCH v5 32/49] KVM: arm64: gic-v5: Handle userspace accesses to IRS MMIO region

[Sascha Bischoff <Sascha.Bischoff@arm.com>]

On Fri, 2026-08-07 at 16:20 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [High] Userspace can inject an inconsistent state by setting
> IRS_IST_BASER.valid=1 without allocating a host IST, leading to a
> CONSTRAINED UNPREDICTABLE hardware state if the guest subsequently
> clears the valid bit.
> --
> 
> --- Patch [32]: [PATCH v5 32/49] KVM: arm64: gic-v5: Handle userspace
> accesses to IRS MMIO region ---
> commit 24ee880222eb3e4ff06c9b27b1d655da134efd7a
> Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
> Date:   Fri Aug 7 11:28:54 2026 +0000
> 
> KVM: arm64: gic-v5: Handle userspace accesses to IRS MMIO region
>     
> As part of saving and restoring the state of a GICv5-based system,
> userspace must save and restore the IRS MMIO registers. These include
> important information such as the guest IST configuration, and KVM
> must present consistent state to the guest after migration.
> 
> > diff --git a/arch/arm64/kvm/vgic/vgic-irs-v5.c
> > b/arch/arm64/kvm/vgic/vgic-irs-v5.c
> > index 095046c7959fa..212066b8d7ed5 100644
> > --- a/arch/arm64/kvm/vgic/vgic-irs-v5.c
> > +++ b/arch/arm64/kvm/vgic/vgic-irs-v5.c
> [ ... ]
> > +static int vgic_v5_mmio_uaccess_write_irs(struct kvm_vcpu *vcpu,
> > gpa_t addr,
> > +					  unsigned int len,
> > unsigned long val)
> > +{
> > +	struct vgic_dist *vgic = &vcpu->kvm->arch.vgic;
> > +	struct vgic_v5_irs *irs_data = vgic->vgic_v5_irs_data;
> > +	size_t offset = addr & (SZ_64K - 1);
> >  
> > -		/* The address cannot be changed while the IST is
> > valid. */
> > -		if (irs->ist_baser.valid && valid)
> > -			return;
> > +	/*
> > +	 * The following registers are ONLY settable via
> > uaccesses. The guest
> > +	 * cannot write them!
> > +	 */
> >  
> > -		/* Valid -> Invalid */
> > -		if (irs->ist_baser.valid && !valid) {
> > -			/* Make the LPI IST invalid and then ...
> > */
> > -			if
> > (irq_set_vcpu_affinity(vgic_v5_vpe_db(vcpu), &cmd))
> > -				break;
> > +	switch (offset) {
> [ ... ]
> > +	case GICV5_IRS_IST_BASER:
> > +		if (irs_data->ist_baser.valid &&
> > +		    !vgic_v5_ist_baser_matches(irs_data, val))
> > +			return -EINVAL;
> > +
> > +		vgic_v5_update_irs_ist_baser(irs_data, val);
> > +		break;
> 
> [Severity: High]
> Does this allow userspace to inject an inconsistent state?
> 
> If userspace sets the valid bit of GICV5_IRS_IST_BASER to 1 via
> vgic_v5_mmio_uaccess_write_irs() but does not actually allocate a
> host LPI IST
> (by omitting the KVM_DEV_ARM_VGIC_GRP_IST ioctl), the VM can still be
> started.
> 
> When the guest subsequently clears the valid bit, KVM's MMIO handler:
> 
> vgic_v5_write_irs_ist_baser() {
>     ...
>     if (irs->ist_baser.valid && !valid) {
>         /* Make the LPI IST invalid and then ... */
>         rc = irq_set_vcpu_affinity(vgic_v5_vpe_db(vcpu), &cmd);
> 
> detects a Valid -> Invalid transition and issues the
> LPI_VIST_MAKE_INVALID
> command. This sends an unmap command to the hardware for a VIST that
> was never
> mapped.
> 
> Since this results in CONSTRAINED UNPREDICTABLE hardware behavior,
> should KVM
> reject uaccess writes that set the valid bit without a backing IST,
> or perhaps
> verify IST presence before issuing the unmap command?
> 

Right, this is an issue. I've added some tracking in such that when
userspace sets the IRS_IST_BASER.Valid bit it must also restore an LPI
IST. Without that, we now fail KVM_RUN. This means that the userspace-
set state must be consistent with the state being provided to the
hardware (via the VMTE) for the guest to be runnable.

This change spans this commit and "KVM: arm64: gic-v5: Implement
save/restore mechanisms for ISTs".

Thanks,
Sascha