Re: [PATCH v4 22/48] KVM: arm64: gic-v5: Register the IRS IODEV
Sascha Bischoff <[email protected]> Fri, 31 Jul 2026 13:05:13 +0000
| Newsgroups | dev.linux.lists.sashiko-reviews,dev.linux.lists.kvmarm,org.kernel.vger.kvm |
|---|---|
| Message-ID | <[email protected]> |
On Fri, 2026-07-24 at 11:33 +0000, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 2 potential > issue(s) to consider: > - [High] GICv5 SPIs cannot be retrieved because vgic_get_irq() > explicitly rejects non-private GICv5 IRQs, rendering the newly > activated IRS SPI configuration broken. > - [High] Allowing userspace to configure up to 65,536 GICv5 SPIs > triggers a large contiguous memory allocation via kcalloc, which can > deterministically fail and cause kernel warnings, resulting in a > host-side DoS. > -- > > --- Patch [22]: [PATCH v4 22/48] KVM: arm64: gic-v5: Register the IRS > IODEV --- > commit d0f49503c9e90ab6ab9f71cb5ba22d59c564bcd8 > Author: Sascha Bischoff <[email protected]> > KVM: arm64: gic-v5: Register the IRS IODEV > > Now that we have an emulated IRS, it needs to be registered, which > ensures that > guest accesses to the MMIO regions handled by the device are handled > appropriately in KVM. As part of this change, expose setting the > address of > the emulated IRS via KVM_VGIC_V5_ADDR_TYPE_IRS to userspace. Also > allow > userspace to set the number of SPIs handled by the emulated GICv5 > implementation. > > > diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c > > b/arch/arm64/kvm/vgic/vgic-kvm-device.c > > index 90be99443df3b..8c2c6886af4e3 100644 > > --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c > > +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c > > [ ... ] > > > @@ -224,31 +232,48 @@ static int vgic_set_common_attr(struct > > kvm_device *dev, > > if (get_user(val, uaddr)) > > return -EFAULT; > > > > - /* > > - * We require: > > - * - at least 32 SPIs on top of the 16 SGIs and 16 > > PPIs > > - * - at most 1024 interrupts > > - * - a multiple of 32 interrupts > > - */ > > - if (val < (VGIC_NR_PRIVATE_IRQS + 32) || > > - val > VGIC_MAX_RESERVED || > > - (val & 31)) > > - return -EINVAL; > > + if (!vgic_is_v5(dev->kvm)) { > > [ ... ] > > > + } else { > > + /* > > + * GICv5 reports a number of SPIs, not a > > total number of > > + * interrupts. Require a multiple of 32 > > SPIs. > > + */ > > + if (val < VGIC_V5_DEFAULT_NR_SPIS || > > + val > VGIC_V5_MAX_NR_SPIS || > > + (val & 31)) > > + return -EINVAL; > > [Severity: High] > Does allowing userspace to set nr_spis up to VGIC_V5_MAX_NR_SPIS > (65536) risk > triggering a host-side allocation warning or failure? > > If userspace sets nr_spis to the maximum, KVM will eventually call > kcalloc() > for the SPIs: > > arch/arm64/kvm/vgic/vgic-irs-v5.c:kvm_vgic_v5_irs_init() { > ... > dist->spis = kcalloc(nr_spis, sizeof(struct vgic_irq), > GFP_KERNEL_ACCOUNT); > ... > } > > Since sizeof(struct vgic_irq) is over 100 bytes, this requires a > physically > contiguous allocation of roughly 7MB. This exceeds the typical > KMALLOC_MAX_SIZE > of 4MB, which would cause the allocation to deterministically fail > and > potentially trigger a kernel warning or panic (if panic_on_warn is > set), > allowing an unprivileged user to cause a host-side denial of service. OK, yes, this would be an issue. Following an offline discussion with Marc, I'm going to cap SPIs for GICv5 at 1024 for the time being. This is a similar upper limit to what we have for GICv3 under KVM, and this has so far not been an issue there. If and when the need arises in the future, this could be bumped up. > > > + > > + mutex_lock(&dev->kvm->arch.config_lock); > > + if (vgic_initialized(dev->kvm) || dev- > > >kvm->arch.vgic.nr_spis) > > + ret = -EBUSY; > > + else > > + dev->kvm->arch.vgic.nr_spis = val; > > [Severity: High] > Are we able to retrieve these GICv5 SPIs once they are configured > here? > > Currently, vgic_get_irq() explicitly rejects non-private GICv5 IRQs: > > arch/arm64/kvm/vgic/vgic.c:vgic_get_irq() { > ... > /* Non-private IRQs are not yet implemented for GICv5 */ > if (vgic_is_v5(kvm)) > return NULL; > ... > } > > This means that when the guest accesses the IRS MMIO interface for > SPIs, the > handler (e.g., vgic_v5_mmio_write_irs_spi) will fail to retrieve the > SPI > structure and silently abort. Will this leave GICv5 SPIs > unconfigurable? With just this commit, yes. The support comes in a later commit. > > > + mutex_unlock(&dev->kvm->arch.config_lock); > > + } > > > > return ret; > > } > Thanks, Sascha