Re: [PATCH v3 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] Unconditional KVM reference overwrite and subsequent premature release in the VFIO cdev bind error path.
--

commit 0e9c12cfda2c8eedbfdfe11e2a9a3b8ef09209f4
Author: Paolo Bonzini <pbonzini@redhat.com>

VFIO: take reference to the KVM module

VFIO implicitly took a reference to the KVM module via symbol_get and
symbol_put. In preparation for removing these, this patch stores a pointer
to the KVM module and uses module_get()/module_put() to keep KVM alive.

> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 6222376ab6ab5..78b9cad348b19 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
[ ... ]
> @@ -490,6 +496,8 @@ void vfio_device_put_kvm(struct vfio_device *device)
>  
>  clear:
>  	device->kvm = NULL;
> +	module_put(device->kvm_module);
> +	device->kvm_module = NULL;
>  }
>  #endif

[Severity: High]
This isn't a bug introduced by this patch, but does this change exacerbate
a bug in the cdev bind error path by adding a module_put() underflow?

In vfio_df_ioctl_bind_iommufd(), if a second bind attempt is made on an
already-open device via a new file descriptor, the code unconditionally
calls vfio_df_get_kvm_safe() before checking device->open_count:

	vfio_df_get_kvm_safe(df);

	ret = vfio_df_open(df);
	if (ret)
		goto out_put_kvm;

If the new descriptor's kvm is populated, vfio_device_get_kvm_safe() blindly
overwrites device->kvm and device->kvm_module, leaking the prior references
held by the first valid bind.

If the new descriptor's kvm is NULL, vfio_device_get_kvm_safe() returns early,
leaving device->kvm still pointing to the first descriptor's KVM.

Then vfio_df_open() predictably fails with -EINVAL because
device->open_count != 0, and the error path jumps to out_put_kvm:

out_put_kvm:
	vfio_device_put_kvm(device);

This drops the KVM references and clears device->kvm to NULL. This destroys
the KVM association for the initially bound, actively running process, and
could lead to a use-after-free of KVM structures.

By adding module_put(device->kvm_module) to vfio_device_put_kvm(), does this
also cause a module reference underflow for the actively running process?

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