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

Tian, Kevin
2023-03-10T05:10:54Z

> From: Liu, Yi L <yi.l.liu@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().

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().