[Intel-gfx] [PATCH v6 11/24] vfio/pci: Accept device fd in VFIO_DEVICE_PCI_HOT_RESET ioctl

[Liu, Yi L]

> From: Tian, Kevin <kevin.tian at intel.com>
> Sent: Friday, March 10, 2023 1:08 PM
> 
> > From: Liu, Yi L <yi.l.liu at intel.com>
> > Sent: Wednesday, March 8, 2023 9:29 PM
> >
> > @@ -1319,8 +1319,14 @@ static int vfio_pci_ioctl_pci_hot_reset(struct
> > vfio_pci_core_device *vdev,
> >  			break;
> >  		}
> >
> > -		/* Ensure the FD is a vfio group FD.*/
> > -		if (!vfio_file_is_group(file)) {
> > +		/*
> > +		 * For vfio group FD, sanitize the file is enough.
> > +		 * For vfio device FD, needs to ensure it has got the
> > +		 * access to device, otherwise it cannot be used as
> > +		 * proof of device ownership.
> > +		 */
> > +		if (!vfio_file_is_valid(file) ||
> > +		    (!vfio_file_is_group(file)
> > && !vfio_file_has_device_access(file))) {
> >  			fput(file);
> >  			ret = -EINVAL;
> >  			break;
> 
> IMHO it's clearer to just check whether it's a valid vfio group/device fd
> here.
> 
> then further restrictions are checked inside vfio_file_has_dev() when
> it's called by vfio_dev_in_user_fds().

I see. But it just has a window in which a device file has not opened
device yet in this check, but opens the device before the dev_set->lock
is held by vfio_pci_dev_set_hot_reset(). Anyhow, no issue. So I can change
it. Then vfio_file_is_group() can be removed.

> 
> if fd is group then check whether device belongs to group.
> 
> if fd is device then check whether device allows access.
> 
> i.e.
> 
> >
> > +/**
> > + * vfio_file_has_device_access - True if the file has opened device
> > + * @file: VFIO device file
> > + */
> > +bool vfio_file_has_device_access(struct file *file)
> > +{
> > +	struct vfio_device_file *df;
> > +
> > +	if (vfio_group_from_file(file) ||
> > +	    !vfio_device_from_file(file))
> > +		return false;
> > +
> > +	df = file->private_data;
> > +
> > +	return READ_ONCE(df->access_granted);
> > +}
> > +EXPORT_SYMBOL_GPL(vfio_file_has_device_access);
> > +
> > +/**
> > + * vfio_file_has_dev - True if the VFIO file is a handle for device
> > + * @file: VFIO file to check
> > + * @device: Device that must be part of the file
> > + *
> > + * Returns true if given file has permission to manipulate the given device.
> > + */
> > +bool vfio_file_has_dev(struct file *file, struct vfio_device *device)
> > +{
> > +	struct vfio_group *group;
> > +	struct vfio_device *vdev;
> > +
> > +	group = vfio_group_from_file(file);
> > +	if (group)
> > +		return vfio_group_has_dev(group, device);
> > +
> > +	vdev = vfio_device_from_file(file);
> > +	if (device)
> > +		return vdev == device;
> > +
> > +	return false;
> > +}
> > +EXPORT_SYMBOL_GPL(vfio_file_has_dev);
> > +
> 
> merge above two into one vfio_file_has_dev().