Re: [PATCH] vfio: cdev: Reject duplicate bind before updating KVM file

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] vfio: UAPI breakage returning -EINVAL instead of -EBUSY on duplicate bind
--

commit c22368f571a5e2ac9cc22bf312688fe247d99cd9
Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>

vfio: cdev: Reject duplicate bind before updating KVM file

This commit prevents a second file descriptor for the same device from
overwriting the KVM reference. It does this by checking device->open_count
earlier during the bind operation.

> diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
> index 30362936c7b59..0f23ccb780e05 100644
> --- a/drivers/vfio/device_cdev.c
> +++ b/drivers/vfio/device_cdev.c
> @@ -124,8 +124,8 @@ long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
>  		return ret;
>  
>  	mutex_lock(&device->dev_set->lock);
> -	/* one device cannot be bound twice */
> -	if (df->access_granted) {
> +	/* The cdev path only supports one bound/open device fd. */
> +	if (df->access_granted || device->open_count) {
>  		ret = -EINVAL;
>  		goto out_unlock;
>  	}

[Severity: High]
Does this change unintentionally alter the UAPI by returning -EINVAL instead
of -EBUSY when device->open_count is non-zero?

Further down in drivers/vfio/device_cdev.c:vfio_df_ioctl_bind_iommufd(),
there is an explicit check intended to handle transient opens:

	if (device->open_count) {
		ret = -EBUSY;
		goto out_unlock;
	}

By checking device->open_count in the first conditional block and returning
-EINVAL, the second block returning -EBUSY becomes dead code. Will this
cause applications relying on -EBUSY to trigger a retry to fail unexpectedly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001093152.1698474-1-aneesh.kumar@kernel.org?part=1