On Tue, Jun 24, 2025 at 02:06:04PM -0600, Alex Williamson wrote:
> > This is used to control access to a VF unless there is co-ordination with
> > the owner of the PF.
> >
> > Since we no longer have a device name pass the token directly though
>
> s/name pass/name, pass/ s/though/through/
Got it
> > @@ -132,6 +132,7 @@ struct vfio_device_ops {
> > int (*mmap)(struct vfio_device *vdev, struct vm_area_struct *vma);
> > void (*request)(struct vfio_device *vdev, unsigned int count);
> > int (*match)(struct vfio_device *vdev, char *buf);
> > + int (*match_token_uuid)(struct vfio_device *vdev, const uuid_t
> > *uuid);
> > void (*dma_unmap)(struct vfio_device *vdev, u64 iova, u64 length);
> > int (*device_feature)(struct vfio_device *device, u32 flags,
> > void __user *arg, size_t argsz);
>
> Update the structure comments.
* @match_token_uuid: Optional device token match/validation. Return 0
* if the uuid is valid for the device, -errno otherwise. uuid is NULL
* if none was provided.
> > diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
> > index fbb472dd99b361..f541044e42a2ad 100644
> > --- a/include/linux/vfio_pci_core.h
> > +++ b/include/linux/vfio_pci_core.h
> > @@ -122,6 +122,8 @@ ssize_t vfio_pci_core_write(struct vfio_device
> > *core_vdev, const char __user *bu
> > int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct
> > vm_area_struct *vma);
> > void vfio_pci_core_request(struct vfio_device *core_vdev, unsigned int
> > count);
> > int vfio_pci_core_match(struct vfio_device *core_vdev, char *buf);
> > +int vfio_pci_core_match_token_uuid(struct vfio_device *core_vdev,
> > + const uuid_t *uuid);
> > int vfio_pci_core_enable(struct vfio_pci_core_device *vdev);
> > void vfio_pci_core_disable(struct vfio_pci_core_device *vdev);
> > void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev);
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index 5764f315137f99..48233ec4daf7b4 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -901,14 +901,18 @@ struct vfio_device_feature {
> >
> > #define VFIO_DEVICE_FEATURE _IO(VFIO_TYPE, VFIO_BASE + 17)
> >
> > +#define VFIO_DEVICE_BIND_TOKEN (1 << 0)
>
> We tend to define ioctl flags within the ioctl data structure and
> include "_FLAG_" in the name.
*/
struct vfio_device_bind_iommufd {
__u32 argsz;
__u32 flags;
#define VFIO_DEVICE_BIND_FLAG_TOKEN (1 << 0)
__s32 iommufd;
> > @@ -924,6 +934,7 @@ struct vfio_device_bind_iommufd {
> > __u32 flags;
> > __s32 iommufd;
> > __u32 out_devid;
> > + __aligned_u64 token_uuid_ptr;
> > };
>
> So we're expecting in the general case, old code doesn't set the flag,
> doesn't need a token, continues to work.
Yes
> There's potentially a narrow case of old code that should have
> required a token, which now intentionally breaks.
Yes
> We're not offering an introspection mechanism
> here, but doing so also doesn't add a lot of value.
Right.
> Userspace needs to know the token to pass anyway. Is that how you
> see it?
Yes, we are fixing a security bug here.
> Do note that QEMU already has support for this in the legacy interface
> and should just need to reparse the token from the name provided
> through the attach_device callback and pass it through to the
> iommufd_cdev_connect_and_bind() function.
Yes, that sounds right.
I will repost it and hopefully someone has an easy test environment
Jason
