Re: [PATCH v2 09/16] KVM: arm64: gic-v5: Add ITS MSI injection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: arm64: gic-v5: missing vgic_initialized() check in MSI injection
--

commit b2f7c0263871635da8ddb0614e8b96f1cb1f641c
Author: Sascha Bischoff <sascha.bischoff@arm.com>

KVM: arm64: gic-v5: Add ITS MSI injection

This commit adds MSI injection support for the virtual GICv5 ITS via both
the slow guest-memory translation path and the inatomic cached translation
path. It wires kvm_set_msi() and kvm_arch_set_irq_inatomic() to invoke the
new GICv5 ITS injection helpers for GICv5 VMs.

> diff --git a/arch/arm64/kvm/vgic/vgic-its-v5.c b/arch/arm64/kvm/vgic/vgic-its-v5.c
> index 9f4783b243221..b69a1d107c0ae 100644
> --- a/arch/arm64/kvm/vgic/vgic-its-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-its-v5.c

[ ... ]

> @@ -424,12 +468,86 @@ static int __vgic_v5_its_resolve_lpi(struct kvm *kvm, struct vgic_v5_its *its,
>  	return ret;
>  }
>  
> +int vgic_v5_its_resolve_lpi(struct kvm *kvm, struct vgic_v5_its *its,
> +			    u32 device_id, u32 event_id,
> +			    struct vgic_irq **irq)
> +{
> +	guard(mutex)(&its->its_lock);
> +
> +	return __vgic_v5_its_resolve_lpi(kvm, its, device_id, event_id, irq);
> +}
> +
>  static void vgic_v5_its_inject_lpi(struct kvm *kvm, struct vgic_irq *irq)
>  {
>  	kvm_call_hyp(__vgic_v5_vdpend, irq->intid, true,
>  		     kvm->arch.vgic.gicv5_vm.vm_id);
>  }
>  
> +int vgic_v5_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
> +{
> +	struct vgic_v5_its *its;
> +	struct vgic_irq *irq;
> +	bool borrowed_mm = false;
> +	int ret;
> +
> +	its = vgic_v5_msi_to_its(kvm, msi);
> +	if (IS_ERR(its))
> +		return PTR_ERR(its);
> +
> +	/* An irqfd cache miss is resolved by a worker outside the VM's mm. */
> +	if (current->mm != kvm->mm) {
> +		if (WARN_ON_ONCE(current->mm || !(current->flags & PF_KTHREAD)))
> +			return -EWOULDBLOCK;
> +
> +		if (!mmget_not_zero(kvm->mm))
> +			return -EFAULT;
> +
> +		kthread_use_mm(kvm->mm);
> +		borrowed_mm = true;
> +	}
> +
> +	ret = vgic_v5_its_resolve_lpi(kvm, its, msi->devid, msi->data, &irq);
> +	if (borrowed_mm) {
> +		kthread_unuse_mm(kvm->mm);
> +		mmput(kvm->mm);
> +	}
> +
> +	if (ret < 0)
> +		return ret;
> +	/* KVM_SIGNAL_MSI reports guest translation failures as blocked MSIs. */
> +	if (ret)
> +		return 0;
> +
> +	vgic_v5_its_inject_lpi(kvm, irq);
> +	vgic_put_irq(kvm, irq);
> +
> +	return 1;
> +}
> +
> +int vgic_v5_its_inject_cached_translation(struct kvm *kvm, struct kvm_msi *msi)
> +{
> +	struct vgic_v5_its *its;
> +	struct vgic_irq *irq;
> +	int ret;
> +
> +	its = vgic_v5_msi_to_its(kvm, msi);
> +	if (IS_ERR(its))
> +		return PTR_ERR(its);
> +
> +	if (!READ_ONCE(its->enabled))
> +		return -EWOULDBLOCK;
> +
> +	ret = its_cache_look_up_translation(its, msi->devid, (u16)msi->data,
> +					    &irq);
> +	if (ret)
> +		return -EWOULDBLOCK;
> +
> +	vgic_v5_its_inject_lpi(kvm, irq);
> +	vgic_put_irq(kvm, irq);
> +
> +	return 0;
> +}

[Severity: High]
Can userspace trigger vgic_v5_its_inject_lpi() before the GICv5 VM has been
initialized?

When a VM creates KVM_DEV_TYPE_ARM_VGIC_V5 and KVM_DEV_TYPE_ARM_VGIC_V5_ITS,
kvm_vgic_create() initializes kvm->arch.vgic.gicv5_vm.vm_id to
VGIC_V5_VM_ID_INVAL (-1), and a valid VM ID is only allocated later during
KVM_DEV_ARM_VGIC_CTRL_INIT via:

  vgic_init()
    vgic_v5_init()
      vgic_v5_allocate_vm_id()

However, vgic_v5_register_its_iodev() registers the ITS on KVM_MMIO_BUS as
soon as KVM_VGIC_V5_ADDR_TYPE_ITS is set, and userspace register access
allows writing GICV5_ITS_CR0 (setting its->enabled = true),
GICV5_ITS_DT_CFGR, and GICV5_ITS_DT_BASER before vgic_init() is called.

If userspace invokes KVM_SIGNAL_MSI or triggers an irqfd before
KVM_DEV_ARM_VGIC_CTRL_INIT, neither __vgic_v5_its_resolve_lpi(),
vgic_v5_its_inject_msi(), nor vgic_v5_its_inject_cached_translation() checks
vgic_initialized(kvm). As a result, vgic_v5_its_inject_lpi() will pass
VGIC_V5_VM_ID_INVAL truncated to u16 (0xffff) to __vgic_v5_vdpend(), which
executes the hardware VDPEND instruction with an unallocated VM ID (or
targets another active VM using VM ID 0xffff).

Should the MSI resolution and injection paths check vgic_initialized(kvm)
before resolving translations or calling vgic_v5_its_inject_lpi()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102194454.4086911-1-sascha.bischoff@arm.com?part=9