Re: [PATCH v5 32/49] KVM: arm64: gic-v5: Handle userspace accesses to IRS MMIO region
[email protected] Fri, 07 Aug 2026 16:20:52 +0000
| Newsgroups | dev.linux.lists.kvmarm,org.kernel.vger.kvm |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> 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? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=32