Re: [PATCH v4 35/48] KVM: arm64: gic-v5: Implement save/restore mechanisms for ISTs

On Fri, 24 Jul 2026 at 11:58, Sascha Bischoff [off-list ref] wrote:
...

+/*
+ * Save the LPI IST to userspace memory.
+ *
+ * The LPI IST may be linear or two-level, so host iteration depends on the
+ * allocated host shape.
+ */
+int vgic_v5_save_lpi_ist(struct kvm *kvm, struct kvm_vgic_v5_ist *ist_attr)
+{
+       struct vgic_v5_ist_desc ist;
+       u32 __user *uaddr;
+       int ret;
+
+       ret = vgic_v5_get_lpi_ist_desc(kvm, &ist);
+       if (ret)
+               return ret;
+
+       if (!ist.present)
+               return 0;
+
+       uaddr = (u32 __user *)(unsigned long)ist_attr->lpi_ist_addr;
+
+       if (!ist.vmi->h_lpi_ist_structure)
+               return vgic_v5_save_linear_ist(&ist, uaddr,
+                                              BIT(ist.id_bits));
+
+       return vgic_v5_save_two_level_ist(&ist, uaddr);
+}

This bounds the walk with the ID_BITS stamped into the VMTE when the
host IST was allocated...

...

+static int vgic_v5_validate_ist_attr(struct kvm *kvm,
+                                    const struct kvm_vgic_v5_ist *ist_attr)
+{
+       unsigned int id_bits;
+       int ret;
+
+       /* We always have SPIs to save */
+       ret = vgic_v5_validate_ist_user_buffer(ist_attr->spi_ist_addr,
+                                       ist_attr->spi_ist_size,
+                                       kvm->arch.vgic.nr_spis * sizeof(__u32));
+       if (ret)
+               return ret;
+
+       /* We don't always have LPIs to save */
+       ret = vgic_v5_irs_lpi_ist_id_bits(kvm, &id_bits);
+       if (ret < 0)
+               return ret;
+
+       /* No LPI IST */
+       if (!ret) {
+               if (ist_attr->lpi_ist_addr || ist_attr->lpi_ist_size)
+                       return -EINVAL;
+
+               return 0;
+       }
+
+       return vgic_v5_validate_ist_user_buffer(ist_attr->lpi_ist_addr,
+                                              ist_attr->lpi_ist_size,
+                                              BIT(id_bits) * sizeof(__u32));
+}

... and this sizes the buffer from the emulated IRS_IST_CFGR. I think
the two are using different values, and I could not find anything that
re-syncs them.

The two writers of IRS_IST_CFGR differ: `vgic_v5_mmio_write_irs_ist()`
returns early while `ist_baser.valid` is set,
`vgic_v5_mmio_uaccess_write_irs()` does not. So userspace can restore
the IST, stamping the VMTE from the larger LPI_ID_BITS, rewrite
IRS_IST_CFGR smaller, and then save with the matching smaller buffer.
`vgic_v5_ist_cfgr_valid()` accepts down to IRS_IDR2.MIN_LPI_ID_BITS,
so on a part reporting 0 that is BIT(16) entries into a buffer
validated at 4 bytes. `test_vgic_v5_ist_save_restore()` in patch 48
already restores and then re-saves before any vCPU runs, it just does
not write IRS_IST_CFGR in that window.

arm-vgic-v5.rst says the LPI buffer size must be BIT(lpi_id_bits) *
sizeof(__u32), "where lpi_id_bits is the LPI ID width configured in
IRS_IST_CFGR", which is the value the validation uses and not the one
the walk uses.

Adding the `ist_baser.valid` check to the uaccess path would break
restore, since the same file says the IRS registers may be restored in
any order. Validating against the value the walk uses, or rejecting a
save when the register and the VMTE disagree,
both look workable. Bounding the walk by the register instead would
read past the host allocation whenever the register is the larger of
the two, so I do not think that direction works.

`put_user()` targets the caller's own mapping and the host-side reads
stay in bounds, so I think this is a correctness problem rather than a
security one. The SPI save uses `nr_spis` on both sides and restore
stamps the VMTE from the value it validated,
so only the LPI save looks affected, linear and two-level alike.

Cheers,
/fuad