Re: [PATCH 1/2] KVM: arm64: vgic: Require matching VGIC model for GICv3 ITS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: vgic: its_model state leak on ITS creation failure prevents VGIC fallback
--

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

KVM: arm64: vgic: Require matching VGIC model for GICv3 ITS

On a GICv5 host with legacy support, KVM registers both VGICv3 and
VGICv5 devices. A VM can create a GICv3 ITS with another VGIC model,
setting has_its even though that model cannot use the GICv3 ITS.

> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> index e5657cb603567..bf901626ea6df 100644
> --- a/arch/arm64/kvm/vgic/vgic-its.c
> +++ b/arch/arm64/kvm/vgic/vgic-its.c

[ ... ]

> @@ -1903,6 +1911,7 @@ static int vgic_its_create(struct kvm_device *dev, u32 type)
>  
>  	dev->kvm->arch.vgic.msis_require_devid = true;
>  	dev->kvm->arch.vgic.has_its = true;
> +	dev->kvm->arch.vgic.its_model = KVM_DEV_TYPE_ARM_VGIC_V3;
>  	its->enabled = false;
>  	its->dev = dev;

[Severity: Medium]
When vgic_its_create() successfully executes, it updates the VM-wide state
(its_model, has_its, msis_require_devid) to reflect the ITS device. If the
generic kvm_ioctl_create_device() subsequently fails at anon_inode_getfd()
due to process FD exhaustion, it aborts the device creation and calls the
vgic_its_destroy() cleanup callback.

However, vgic_its_destroy() does not roll back the its_model VM-wide property:

arch/arm64/kvm/vgic/vgic-its.c:vgic_its_destroy() {
    ...
	vgic_its_free_device_list(kvm, its);
	vgic_its_free_collection_list(kvm, its);
	vgic_its_invalidate_cache(its);
	xa_destroy(&its->translation_cache);

	mutex_unlock(&its->its_lock);
	kfree(its);
	kfree(kvm_dev);
}

Consequently, the VM permanently records that it requires a GICv3 VGIC model.

Does this state inconsistency prevent userspace from recovering and falling
back to a different VGIC model (such as VGICv5) after an ephemeral FD
allocation error, or if userspace intentionally closes the ITS file descriptor
before creating the VGIC?

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