Re: [PATCH v4 01/27] VFIO: take reference to the KVM module

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] In the VFIO cdev binding path, concurrent or repeated calls to `VFIO_DEVICE_BIND_IOMMUFD` on multiple file descriptors for the same device cause KVM reference leaks and state corruption.
--

commit 298282ceb4d6390cae8f782bcdd7f84408cf4d30
Author: Paolo Bonzini <pbonzini@redhat.com>

VFIO: take reference to the KVM module

This patch modifies VFIO to store a pointer to the KVM module alongside the
KVM pointer. It updates the get/set paths to take and release a module
reference to prevent the KVM module from unloading while in use, in
preparation for removing symbol_get and symbol_put.

> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index ed538aebb0b8e..8289aca97b6be 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
[ ... ]
> @@ -448,7 +448,8 @@ void vfio_unregister_group_dev(struct vfio_device *device)
>  EXPORT_SYMBOL_GPL(vfio_unregister_group_dev);
>  
>  #if IS_ENABLED(CONFIG_KVM)
> -void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm)
> +void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm,
> +			      struct module *kvm_module)
>  {
>  	void (*pfn)(struct kvm *kvm);
>  	bool (*fn)(struct kvm *kvm);
> @@ -459,25 +460,31 @@ void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm)
>  	if (!kvm)
>  		return;
>  
> +	if (!try_module_get(kvm_module))
> +		return;
> +
>  	pfn = symbol_get(kvm_put_kvm);
>  	if (WARN_ON(!pfn))
> -		return;
> +		goto out_put_mod;
>  
>  	fn = symbol_get(kvm_get_kvm_safe);
> -	if (WARN_ON(!fn)) {
> -		symbol_put(kvm_put_kvm);
> -		return;
> -	}
> +	if (WARN_ON(!fn))
> +		goto out_put_sym;
>  
>  	ret = fn(kvm);
>  	symbol_put(kvm_get_kvm_safe);
> -	if (!ret) {
> -		symbol_put(kvm_put_kvm);
> -		return;
> -	}
> +	if (!ret)
> +		goto out_put_sym;
>  
>  	device->put_kvm = pfn;
>  	device->kvm = kvm;
> +	device->kvm_module = kvm_module;
> +	return;

[Severity: High]
This is a pre-existing issue, but can concurrent or repeated calls to
VFIO_DEVICE_BIND_IOMMUFD on multiple file descriptors for the same device
cause KVM reference leaks and state corruption here?

In vfio_device_get_kvm_safe() in drivers/vfio/vfio_main.c, the assignments
above blindly overwrite device->kvm (and now device->kvm_module) without
dropping any existing references.

If a user opens the VFIO device node twice (fd1 and fd2) and binds fd1,
device->kvm is set and the reference is taken. If they then call
VFIO_DEVICE_BIND_IOMMUFD on fd2, this function overwrites the pointers,
leaking the initial KVM and module references.

Furthermore, when vfio_df_open() subsequently fails in the fd2 bind path
because device->open_count > 0, the error path in vfio_df_ioctl_bind_iommufd()
unconditionally calls vfio_device_put_kvm(device). This drops the newly
overwritten reference and clears device->kvm to NULL, entirely breaking the
KVM integration for the still-active fd1.

> +
> +out_put_sym:
> +	symbol_put(kvm_put_kvm);
> +out_put_mod:
> +	module_put(kvm_module);
>  }
>  
>  void vfio_device_put_kvm(struct vfio_device *device)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706085229.979525-1-seiden@linux.ibm.com?part=1