> From: Tian, Kevin <kevin.tian@xxxxxxxxx>
> Sent: Friday, March 10, 2023 1:08 PM
>
> > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > 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().
