oe-kbuild-all.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
       [not found] <0-v3-bdd8716e85fe+3978a-vfio_token_jgg@nvidia.com>
@ 2025-07-15 22:55 ` Dan Carpenter
  2025-07-15 23:06   ` Jason Gunthorpe
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-07-15 22:55 UTC (permalink / raw)
  To: oe-kbuild, Jason Gunthorpe, Alex Williamson, Ankit Agrawal,
	Brett Creeley, Giovanni Cabiddu, Kevin Tian, kvm, Longfang Liu,
	qat-linux, Shameer Kolothum, virtualization, Xin Zeng,
	Yishai Hadas
  Cc: lkp, oe-kbuild-all, patches

Hi Jason,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Jason-Gunthorpe/vfio-pci-Do-vf_token-checks-for-VFIO_DEVICE_BIND_IOMMUFD/20250715-001209
base:   32b2d3a57e26804ca96d82a222667ac0fa226cb7
patch link:    https://lore.kernel.org/r/0-v3-bdd8716e85fe%2B3978a-vfio_token_jgg%40nvidia.com
patch subject: [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
config: openrisc-randconfig-r071-20250715 (https://download.01.org/0day-ci/archive/20250716/202507160254.dAjYAz9h-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 15.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202507160254.dAjYAz9h-lkp@intel.com/

smatch warnings:
drivers/vfio/device_cdev.c:126 vfio_df_ioctl_bind_iommufd() warn: missing unwind goto?
drivers/vfio/device_cdev.c:170 vfio_df_ioctl_bind_iommufd() warn: inconsistent returns '&device->dev_set->lock'.

vim +126 drivers/vfio/device_cdev.c

5fcc26969a164e Yi Liu          2023-07-18   83  long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
5fcc26969a164e Yi Liu          2023-07-18   84  				struct vfio_device_bind_iommufd __user *arg)
5fcc26969a164e Yi Liu          2023-07-18   85  {
be2e70b96c3e54 Jason Gunthorpe 2025-07-14   86  	const u32 VALID_FLAGS = VFIO_DEVICE_BIND_FLAG_TOKEN;
5fcc26969a164e Yi Liu          2023-07-18   87  	struct vfio_device *device = df->device;
5fcc26969a164e Yi Liu          2023-07-18   88  	struct vfio_device_bind_iommufd bind;
5fcc26969a164e Yi Liu          2023-07-18   89  	unsigned long minsz;
be2e70b96c3e54 Jason Gunthorpe 2025-07-14   90  	u32 user_size;
5fcc26969a164e Yi Liu          2023-07-18   91  	int ret;
5fcc26969a164e Yi Liu          2023-07-18   92  
5fcc26969a164e Yi Liu          2023-07-18   93  	static_assert(__same_type(arg->out_devid, df->devid));
5fcc26969a164e Yi Liu          2023-07-18   94  
5fcc26969a164e Yi Liu          2023-07-18   95  	minsz = offsetofend(struct vfio_device_bind_iommufd, out_devid);
5fcc26969a164e Yi Liu          2023-07-18   96  
be2e70b96c3e54 Jason Gunthorpe 2025-07-14   97  	ret = get_user(user_size, &arg->argsz);
be2e70b96c3e54 Jason Gunthorpe 2025-07-14   98  	if (ret)
be2e70b96c3e54 Jason Gunthorpe 2025-07-14   99  		return ret;
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  100  	if (user_size < minsz)
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  101  		return -EINVAL;
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  102  	ret = copy_struct_from_user(&bind, minsz, arg, user_size);
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  103  	if (ret)
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  104  		return ret;
5fcc26969a164e Yi Liu          2023-07-18  105  
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  106  	if (bind.iommufd < 0 || bind.flags & ~VALID_FLAGS)
5fcc26969a164e Yi Liu          2023-07-18  107  		return -EINVAL;
5fcc26969a164e Yi Liu          2023-07-18  108  
5fcc26969a164e Yi Liu          2023-07-18  109  	/* BIND_IOMMUFD only allowed for cdev fds */
5fcc26969a164e Yi Liu          2023-07-18  110  	if (df->group)
5fcc26969a164e Yi Liu          2023-07-18  111  		return -EINVAL;
5fcc26969a164e Yi Liu          2023-07-18  112  
5fcc26969a164e Yi Liu          2023-07-18  113  	ret = vfio_device_block_group(device);
5fcc26969a164e Yi Liu          2023-07-18  114  	if (ret)
5fcc26969a164e Yi Liu          2023-07-18  115  		return ret;
5fcc26969a164e Yi Liu          2023-07-18  116  
5fcc26969a164e Yi Liu          2023-07-18  117  	mutex_lock(&device->dev_set->lock);
5fcc26969a164e Yi Liu          2023-07-18  118  	/* one device cannot be bound twice */
5fcc26969a164e Yi Liu          2023-07-18  119  	if (df->access_granted) {
5fcc26969a164e Yi Liu          2023-07-18  120  		ret = -EINVAL;
5fcc26969a164e Yi Liu          2023-07-18  121  		goto out_unlock;
5fcc26969a164e Yi Liu          2023-07-18  122  	}
5fcc26969a164e Yi Liu          2023-07-18  123  
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  124  	ret = vfio_df_check_token(device, &bind);
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  125  	if (ret)
be2e70b96c3e54 Jason Gunthorpe 2025-07-14 @126  		return ret;

This needs to be a goto unlock.

be2e70b96c3e54 Jason Gunthorpe 2025-07-14  127  
5fcc26969a164e Yi Liu          2023-07-18  128  	df->iommufd = iommufd_ctx_from_fd(bind.iommufd);
5fcc26969a164e Yi Liu          2023-07-18  129  	if (IS_ERR(df->iommufd)) {
5fcc26969a164e Yi Liu          2023-07-18  130  		ret = PTR_ERR(df->iommufd);
5fcc26969a164e Yi Liu          2023-07-18  131  		df->iommufd = NULL;
5fcc26969a164e Yi Liu          2023-07-18  132  		goto out_unlock;
5fcc26969a164e Yi Liu          2023-07-18  133  	}
5fcc26969a164e Yi Liu          2023-07-18  134  
5fcc26969a164e Yi Liu          2023-07-18  135  	/*
5fcc26969a164e Yi Liu          2023-07-18  136  	 * Before the device open, get the KVM pointer currently
5fcc26969a164e Yi Liu          2023-07-18  137  	 * associated with the device file (if there is) and obtain
5fcc26969a164e Yi Liu          2023-07-18  138  	 * a reference.  This reference is held until device closed.
5fcc26969a164e Yi Liu          2023-07-18  139  	 * Save the pointer in the device for use by drivers.
5fcc26969a164e Yi Liu          2023-07-18  140  	 */
5fcc26969a164e Yi Liu          2023-07-18  141  	vfio_df_get_kvm_safe(df);
5fcc26969a164e Yi Liu          2023-07-18  142  
5fcc26969a164e Yi Liu          2023-07-18  143  	ret = vfio_df_open(df);
5fcc26969a164e Yi Liu          2023-07-18  144  	if (ret)
5fcc26969a164e Yi Liu          2023-07-18  145  		goto out_put_kvm;
5fcc26969a164e Yi Liu          2023-07-18  146  
5fcc26969a164e Yi Liu          2023-07-18  147  	ret = copy_to_user(&arg->out_devid, &df->devid,
5fcc26969a164e Yi Liu          2023-07-18  148  			   sizeof(df->devid)) ? -EFAULT : 0;
5fcc26969a164e Yi Liu          2023-07-18  149  	if (ret)
5fcc26969a164e Yi Liu          2023-07-18  150  		goto out_close_device;
5fcc26969a164e Yi Liu          2023-07-18  151  
5fcc26969a164e Yi Liu          2023-07-18  152  	device->cdev_opened = true;
5fcc26969a164e Yi Liu          2023-07-18  153  	/*
5fcc26969a164e Yi Liu          2023-07-18  154  	 * Paired with smp_load_acquire() in vfio_device_fops::ioctl/
5fcc26969a164e Yi Liu          2023-07-18  155  	 * read/write/mmap
5fcc26969a164e Yi Liu          2023-07-18  156  	 */
5fcc26969a164e Yi Liu          2023-07-18  157  	smp_store_release(&df->access_granted, true);
5fcc26969a164e Yi Liu          2023-07-18  158  	mutex_unlock(&device->dev_set->lock);
5fcc26969a164e Yi Liu          2023-07-18  159  	return 0;
5fcc26969a164e Yi Liu          2023-07-18  160  
5fcc26969a164e Yi Liu          2023-07-18  161  out_close_device:
5fcc26969a164e Yi Liu          2023-07-18  162  	vfio_df_close(df);
5fcc26969a164e Yi Liu          2023-07-18  163  out_put_kvm:
5fcc26969a164e Yi Liu          2023-07-18  164  	vfio_device_put_kvm(device);
5fcc26969a164e Yi Liu          2023-07-18  165  	iommufd_ctx_put(df->iommufd);
5fcc26969a164e Yi Liu          2023-07-18  166  	df->iommufd = NULL;
5fcc26969a164e Yi Liu          2023-07-18  167  out_unlock:
5fcc26969a164e Yi Liu          2023-07-18  168  	mutex_unlock(&device->dev_set->lock);
5fcc26969a164e Yi Liu          2023-07-18  169  	vfio_device_unblock_group(device);
5fcc26969a164e Yi Liu          2023-07-18 @170  	return ret;
5fcc26969a164e Yi Liu          2023-07-18  171  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-07-15 22:55 ` [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD Dan Carpenter
@ 2025-07-15 23:06   ` Jason Gunthorpe
  2025-07-16 17:00     ` Alex Williamson
  0 siblings, 1 reply; 3+ messages in thread
From: Jason Gunthorpe @ 2025-07-15 23:06 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, Alex Williamson, Ankit Agrawal, Brett Creeley,
	Giovanni Cabiddu, Kevin Tian, kvm, Longfang Liu, qat-linux,
	Shameer Kolothum, virtualization, Xin Zeng, Yishai Hadas, lkp,
	oe-kbuild-all, patches

On Wed, Jul 16, 2025 at 01:55:45AM +0300, Dan Carpenter wrote:
> 5fcc26969a164e Yi Liu          2023-07-18  117  	mutex_lock(&device->dev_set->lock);
> 5fcc26969a164e Yi Liu          2023-07-18  118  	/* one device cannot be bound twice */
> 5fcc26969a164e Yi Liu          2023-07-18  119  	if (df->access_granted) {
> 5fcc26969a164e Yi Liu          2023-07-18  120  		ret = -EINVAL;
> 5fcc26969a164e Yi Liu          2023-07-18  121  		goto out_unlock;
> 5fcc26969a164e Yi Liu          2023-07-18  122  	}
> 5fcc26969a164e Yi Liu          2023-07-18  123  
> be2e70b96c3e54 Jason Gunthorpe 2025-07-14  124  	ret = vfio_df_check_token(device, &bind);
> be2e70b96c3e54 Jason Gunthorpe 2025-07-14  125  	if (ret)
> be2e70b96c3e54 Jason Gunthorpe 2025-07-14 @126  		return ret;
> 
> This needs to be a goto unlock.

Oop yes, thank you

Alex can you fix it up when applying?

Thanks,
Jason

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-07-15 23:06   ` Jason Gunthorpe
@ 2025-07-16 17:00     ` Alex Williamson
  0 siblings, 0 replies; 3+ messages in thread
From: Alex Williamson @ 2025-07-16 17:00 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Dan Carpenter, oe-kbuild, Ankit Agrawal, Brett Creeley,
	Giovanni Cabiddu, Kevin Tian, kvm, Longfang Liu, qat-linux,
	Shameer Kolothum, virtualization, Xin Zeng, Yishai Hadas, lkp,
	oe-kbuild-all, patches

On Tue, 15 Jul 2025 20:06:18 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Wed, Jul 16, 2025 at 01:55:45AM +0300, Dan Carpenter wrote:
> > 5fcc26969a164e Yi Liu          2023-07-18  117  	mutex_lock(&device->dev_set->lock);
> > 5fcc26969a164e Yi Liu          2023-07-18  118  	/* one device cannot be bound twice */
> > 5fcc26969a164e Yi Liu          2023-07-18  119  	if (df->access_granted) {
> > 5fcc26969a164e Yi Liu          2023-07-18  120  		ret = -EINVAL;
> > 5fcc26969a164e Yi Liu          2023-07-18  121  		goto out_unlock;
> > 5fcc26969a164e Yi Liu          2023-07-18  122  	}
> > 5fcc26969a164e Yi Liu          2023-07-18  123  
> > be2e70b96c3e54 Jason Gunthorpe 2025-07-14  124  	ret = vfio_df_check_token(device, &bind);
> > be2e70b96c3e54 Jason Gunthorpe 2025-07-14  125  	if (ret)
> > be2e70b96c3e54 Jason Gunthorpe 2025-07-14 @126  		return ret;
> > 
> > This needs to be a goto unlock.  
> 
> Oop yes, thank you
> 
> Alex can you fix it up when applying?

Yes, I'll apply with:

diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
index 53a602563f00..480cac3a0c27 100644
--- a/drivers/vfio/device_cdev.c
+++ b/drivers/vfio/device_cdev.c
@@ -123,7 +123,7 @@ long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
 
 	ret = vfio_df_check_token(device, &bind);
 	if (ret)
-		return ret;
+		goto out_unlock;
 
 	df->iommufd = iommufd_ctx_from_fd(bind.iommufd);
 	if (IS_ERR(df->iommufd)) {


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-16 17:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <0-v3-bdd8716e85fe+3978a-vfio_token_jgg@nvidia.com>
2025-07-15 22:55 ` [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD Dan Carpenter
2025-07-15 23:06   ` Jason Gunthorpe
2025-07-16 17:00     ` Alex Williamson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).