kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
@ 2025-06-24 18:34 Jason Gunthorpe
  2025-06-24 20:06 ` Alex Williamson
  2025-07-03  6:40 ` Tian, Kevin
  0 siblings, 2 replies; 6+ messages in thread
From: Jason Gunthorpe @ 2025-06-24 18:34 UTC (permalink / raw)
  To: Ankit Agrawal, Brett Creeley, Giovanni Cabiddu, Kevin Tian, kvm,
	Longfang Liu, qat-linux, virtualization, Xin Zeng, Yishai Hadas
  Cc: Alex Williamson, Matthew Rosato, Nicolin Chen, patches,
	Shameer Kolothum, Terrence Xu, Yanting Jiang, Yi Liu,
	Zhenzhong Duan

This was missed during the initial implementation. The VFIO PCI encodes
the vf_token inside the device name when opening the device from the group
FD, something like:

  "0000:04:10.0 vf_token=bd8d9d2b-5a5f-4f5a-a211-f591514ba1f3"

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
VFIO_DEVICE_BIND_IOMMUFD with an optional field indicated by
VFIO_DEVICE_BIND_TOKEN. Only users using a PCI SRIOV VF will need to
provide this. This is done in the usual backwards compatible way.

Fixes: 5fcc26969a16 ("vfio: Add VFIO_DEVICE_BIND_IOMMUFD")
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/vfio/device_cdev.c                    | 38 +++++++++++++++++--
 .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c    |  1 +
 drivers/vfio/pci/mlx5/main.c                  |  1 +
 drivers/vfio/pci/nvgrace-gpu/main.c           |  2 +
 drivers/vfio/pci/pds/vfio_dev.c               |  1 +
 drivers/vfio/pci/qat/main.c                   |  1 +
 drivers/vfio/pci/vfio_pci.c                   |  1 +
 drivers/vfio/pci/vfio_pci_core.c              | 22 +++++++----
 drivers/vfio/pci/virtio/main.c                |  3 ++
 include/linux/vfio.h                          |  1 +
 include/linux/vfio_pci_core.h                 |  2 +
 include/uapi/linux/vfio.h                     | 13 ++++++-
 12 files changed, 74 insertions(+), 12 deletions(-)

I wrote this quickly and don't have an environment available to check it out
on.. Since it feels like a significant miss I felt we should start looking at
it.

diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
index 281a8dc3ed4974..c5d74c7e71f585 100644
--- a/drivers/vfio/device_cdev.c
+++ b/drivers/vfio/device_cdev.c
@@ -60,22 +60,50 @@ static void vfio_df_get_kvm_safe(struct vfio_device_file *df)
 	spin_unlock(&df->kvm_ref_lock);
 }
 
+static int vfio_df_check_token(struct vfio_device *device,
+			       const struct vfio_device_bind_iommufd *bind)
+{
+	uuid_t uuid;
+
+	if (!device->ops->match_token_uuid) {
+		if (bind->flags & VFIO_DEVICE_BIND_TOKEN)
+			return -EINVAL;
+		return 0;
+	}
+
+	if (!(bind->flags & VFIO_DEVICE_BIND_TOKEN))
+		return device->ops->match_token_uuid(device, NULL);
+
+	if (copy_from_user(&uuid, u64_to_user_ptr(bind->token_uuid_ptr),
+			   sizeof(uuid)))
+		return -EFAULT;
+	return device->ops->match_token_uuid(device, &uuid);
+}
+
 long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
 				struct vfio_device_bind_iommufd __user *arg)
 {
+	const u32 VALID_FLAGS = VFIO_DEVICE_BIND_TOKEN;
 	struct vfio_device *device = df->device;
 	struct vfio_device_bind_iommufd bind;
 	unsigned long minsz;
+	u32 user_size;
 	int ret;
 
 	static_assert(__same_type(arg->out_devid, df->devid));
 
 	minsz = offsetofend(struct vfio_device_bind_iommufd, out_devid);
 
-	if (copy_from_user(&bind, arg, minsz))
-		return -EFAULT;
+	ret = get_user(user_size, &arg->argsz);
+	if (ret)
+		return ret;
+	if (bind.argsz < minsz)
+		return -EINVAL;
+	ret = copy_struct_from_user(&bind, minsz, arg, user_size);
+	if (ret)
+		return ret;
 
-	if (bind.argsz < minsz || bind.flags || bind.iommufd < 0)
+	if (bind.iommufd < 0 || bind.flags & ~VALID_FLAGS)
 		return -EINVAL;
 
 	/* BIND_IOMMUFD only allowed for cdev fds */
@@ -93,6 +121,10 @@ long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
 		goto out_unlock;
 	}
 
+	ret = vfio_df_check_token(device, &bind);
+	if (ret)
+		return ret;
+
 	df->iommufd = iommufd_ctx_from_fd(bind.iommufd);
 	if (IS_ERR(df->iommufd)) {
 		ret = PTR_ERR(df->iommufd);
diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
index 2149f49aeec7f8..397f5e44513639 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
@@ -1583,6 +1583,7 @@ static const struct vfio_device_ops hisi_acc_vfio_pci_ops = {
 	.mmap = vfio_pci_core_mmap,
 	.request = vfio_pci_core_request,
 	.match = vfio_pci_core_match,
+	.match_token_uuid = vfio_pci_core_match_token_uuid,
 	.bind_iommufd = vfio_iommufd_physical_bind,
 	.unbind_iommufd = vfio_iommufd_physical_unbind,
 	.attach_ioas = vfio_iommufd_physical_attach_ioas,
diff --git a/drivers/vfio/pci/mlx5/main.c b/drivers/vfio/pci/mlx5/main.c
index 93f894fe60d221..7ec47e736a8e5a 100644
--- a/drivers/vfio/pci/mlx5/main.c
+++ b/drivers/vfio/pci/mlx5/main.c
@@ -1372,6 +1372,7 @@ static const struct vfio_device_ops mlx5vf_pci_ops = {
 	.mmap = vfio_pci_core_mmap,
 	.request = vfio_pci_core_request,
 	.match = vfio_pci_core_match,
+	.match_token_uuid = vfio_pci_core_match_token_uuid,
 	.bind_iommufd = vfio_iommufd_physical_bind,
 	.unbind_iommufd = vfio_iommufd_physical_unbind,
 	.attach_ioas = vfio_iommufd_physical_attach_ioas,
diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c
index e5ac39c4cc6b6f..d95761dcdd58c4 100644
--- a/drivers/vfio/pci/nvgrace-gpu/main.c
+++ b/drivers/vfio/pci/nvgrace-gpu/main.c
@@ -696,6 +696,7 @@ static const struct vfio_device_ops nvgrace_gpu_pci_ops = {
 	.mmap		= nvgrace_gpu_mmap,
 	.request	= vfio_pci_core_request,
 	.match		= vfio_pci_core_match,
+	.match_token_uuid = vfio_pci_core_match_token_uuid,
 	.bind_iommufd	= vfio_iommufd_physical_bind,
 	.unbind_iommufd	= vfio_iommufd_physical_unbind,
 	.attach_ioas	= vfio_iommufd_physical_attach_ioas,
@@ -715,6 +716,7 @@ static const struct vfio_device_ops nvgrace_gpu_pci_core_ops = {
 	.mmap		= vfio_pci_core_mmap,
 	.request	= vfio_pci_core_request,
 	.match		= vfio_pci_core_match,
+	.match_token_uuid = vfio_pci_core_match_token_uuid,
 	.bind_iommufd	= vfio_iommufd_physical_bind,
 	.unbind_iommufd	= vfio_iommufd_physical_unbind,
 	.attach_ioas	= vfio_iommufd_physical_attach_ioas,
diff --git a/drivers/vfio/pci/pds/vfio_dev.c b/drivers/vfio/pci/pds/vfio_dev.c
index 76a80ae7087b51..5731e6856deaf1 100644
--- a/drivers/vfio/pci/pds/vfio_dev.c
+++ b/drivers/vfio/pci/pds/vfio_dev.c
@@ -201,6 +201,7 @@ static const struct vfio_device_ops pds_vfio_ops = {
 	.mmap = vfio_pci_core_mmap,
 	.request = vfio_pci_core_request,
 	.match = vfio_pci_core_match,
+	.match_token_uuid = vfio_pci_core_match_token_uuid,
 	.bind_iommufd = vfio_iommufd_physical_bind,
 	.unbind_iommufd = vfio_iommufd_physical_unbind,
 	.attach_ioas = vfio_iommufd_physical_attach_ioas,
diff --git a/drivers/vfio/pci/qat/main.c b/drivers/vfio/pci/qat/main.c
index 845ed15b67718c..5cce6b0b8d2f3e 100644
--- a/drivers/vfio/pci/qat/main.c
+++ b/drivers/vfio/pci/qat/main.c
@@ -614,6 +614,7 @@ static const struct vfio_device_ops qat_vf_pci_ops = {
 	.mmap = vfio_pci_core_mmap,
 	.request = vfio_pci_core_request,
 	.match = vfio_pci_core_match,
+	.match_token_uuid = vfio_pci_core_match_token_uuid,
 	.bind_iommufd = vfio_iommufd_physical_bind,
 	.unbind_iommufd = vfio_iommufd_physical_unbind,
 	.attach_ioas = vfio_iommufd_physical_attach_ioas,
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 5ba39f7623bb76..ac10f14417f2f3 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -138,6 +138,7 @@ static const struct vfio_device_ops vfio_pci_ops = {
 	.mmap		= vfio_pci_core_mmap,
 	.request	= vfio_pci_core_request,
 	.match		= vfio_pci_core_match,
+	.match_token_uuid = vfio_pci_core_match_token_uuid,
 	.bind_iommufd	= vfio_iommufd_physical_bind,
 	.unbind_iommufd	= vfio_iommufd_physical_unbind,
 	.attach_ioas	= vfio_iommufd_physical_attach_ioas,
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 6328c3a05bcdd4..086aa37a60a2b5 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1821,9 +1821,13 @@ void vfio_pci_core_request(struct vfio_device *core_vdev, unsigned int count)
 }
 EXPORT_SYMBOL_GPL(vfio_pci_core_request);
 
-static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
-				      bool vf_token, uuid_t *uuid)
+int vfio_pci_core_match_token_uuid(struct vfio_device *core_vdev,
+				   const uuid_t *uuid)
+
 {
+	struct vfio_pci_core_device *vdev =
+		container_of(core_vdev, struct vfio_pci_core_device, vdev);
+
 	/*
 	 * There's always some degree of trust or collaboration between SR-IOV
 	 * PF and VFs, even if just that the PF hosts the SR-IOV capability and
@@ -1854,7 +1858,7 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
 		bool match;
 
 		if (!pf_vdev) {
-			if (!vf_token)
+			if (!uuid)
 				return 0; /* PF is not vfio-pci, no VF token */
 
 			pci_info_ratelimited(vdev->pdev,
@@ -1862,7 +1866,7 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
 			return -EINVAL;
 		}
 
-		if (!vf_token) {
+		if (!uuid) {
 			pci_info_ratelimited(vdev->pdev,
 				"VF token required to access device\n");
 			return -EACCES;
@@ -1880,7 +1884,7 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
 	} else if (vdev->vf_token) {
 		mutex_lock(&vdev->vf_token->lock);
 		if (vdev->vf_token->users) {
-			if (!vf_token) {
+			if (!uuid) {
 				mutex_unlock(&vdev->vf_token->lock);
 				pci_info_ratelimited(vdev->pdev,
 					"VF token required to access device\n");
@@ -1893,12 +1897,12 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
 					"Incorrect VF token provided for device\n");
 				return -EACCES;
 			}
-		} else if (vf_token) {
+		} else if (uuid) {
 			uuid_copy(&vdev->vf_token->uuid, uuid);
 		}
 
 		mutex_unlock(&vdev->vf_token->lock);
-	} else if (vf_token) {
+	} else if (uuid) {
 		pci_info_ratelimited(vdev->pdev,
 			"VF token incorrectly provided, not a PF or VF\n");
 		return -EINVAL;
@@ -1906,6 +1910,7 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(vfio_pci_core_match_token_uuid);
 
 #define VF_TOKEN_ARG "vf_token="
 
@@ -1952,7 +1957,8 @@ int vfio_pci_core_match(struct vfio_device *core_vdev, char *buf)
 		}
 	}
 
-	ret = vfio_pci_validate_vf_token(vdev, vf_token, &uuid);
+	ret = vfio_pci_core_match_token_uuid(core_vdev,
+					     vf_token ? &uuid : NULL);
 	if (ret)
 		return ret;
 
diff --git a/drivers/vfio/pci/virtio/main.c b/drivers/vfio/pci/virtio/main.c
index 515fe1b9f94d80..8084f3e36a9f70 100644
--- a/drivers/vfio/pci/virtio/main.c
+++ b/drivers/vfio/pci/virtio/main.c
@@ -94,6 +94,7 @@ static const struct vfio_device_ops virtiovf_vfio_pci_lm_ops = {
 	.mmap = vfio_pci_core_mmap,
 	.request = vfio_pci_core_request,
 	.match = vfio_pci_core_match,
+	.match_token_uuid = vfio_pci_core_match_token_uuid,
 	.bind_iommufd = vfio_iommufd_physical_bind,
 	.unbind_iommufd = vfio_iommufd_physical_unbind,
 	.attach_ioas = vfio_iommufd_physical_attach_ioas,
@@ -114,6 +115,7 @@ static const struct vfio_device_ops virtiovf_vfio_pci_tran_lm_ops = {
 	.mmap = vfio_pci_core_mmap,
 	.request = vfio_pci_core_request,
 	.match = vfio_pci_core_match,
+	.match_token_uuid = vfio_pci_core_match_token_uuid,
 	.bind_iommufd = vfio_iommufd_physical_bind,
 	.unbind_iommufd = vfio_iommufd_physical_unbind,
 	.attach_ioas = vfio_iommufd_physical_attach_ioas,
@@ -134,6 +136,7 @@ static const struct vfio_device_ops virtiovf_vfio_pci_ops = {
 	.mmap = vfio_pci_core_mmap,
 	.request = vfio_pci_core_request,
 	.match = vfio_pci_core_match,
+	.match_token_uuid = vfio_pci_core_match_token_uuid,
 	.bind_iommufd = vfio_iommufd_physical_bind,
 	.unbind_iommufd = vfio_iommufd_physical_unbind,
 	.attach_ioas = vfio_iommufd_physical_attach_ioas,
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 707b00772ce1ff..b2cca540a6b4bf 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -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);
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)
+
 /*
  * VFIO_DEVICE_BIND_IOMMUFD - _IOR(VFIO_TYPE, VFIO_BASE + 18,
  *				   struct vfio_device_bind_iommufd)
  * @argsz:	 User filled size of this data.
- * @flags:	 Must be 0.
+ * @flags:	 Must be 0 or a bit flags of VFIO_DEVICE_BIND_*
  * @iommufd:	 iommufd to bind.
  * @out_devid:	 The device id generated by this bind. devid is a handle for
  *		 this device/iommufd bond and can be used in IOMMUFD commands.
+ * @token_uuid_ptr: Valid if VFIO_DEVICE_BIND_TOKEN. Points to a 16 byte UUID
+ *                  in the same format as VFIO_DEVICE_FEATURE_PCI_VF_TOKEN.
  *
  * Bind a vfio_device to the specified iommufd.
  *
@@ -917,6 +921,12 @@ struct vfio_device_feature {
  *
  * Unbind is automatically conducted when device fd is closed.
  *
+ * A token is sometimes required to open the device, unless this is known to be
+ * needed VFIO_DEVICE_BIND_TOKEN should not be set and token_uuid_ptr is
+ * ignored. The only case today is a PF/VF relationship where the VF bind must
+ * be provided the same token as VFIO_DEVICE_FEATURE_PCI_VF_TOKEN provided to
+ * the PF.
+ *
  * Return: 0 on success, -errno on failure.
  */
 struct vfio_device_bind_iommufd {
@@ -924,6 +934,7 @@ struct vfio_device_bind_iommufd {
 	__u32		flags;
 	__s32		iommufd;
 	__u32		out_devid;
+	__aligned_u64	token_uuid_ptr;
 };
 
 #define VFIO_DEVICE_BIND_IOMMUFD	_IO(VFIO_TYPE, VFIO_BASE + 18)

base-commit: 3e2a9811f6a9cefd310cc33cab73d5435b4a4caa
-- 
2.43.0


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

* Re: [PATCH] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-06-24 18:34 [PATCH] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD Jason Gunthorpe
@ 2025-06-24 20:06 ` Alex Williamson
  2025-07-02 14:56   ` Jason Gunthorpe
  2025-07-03  6:40 ` Tian, Kevin
  1 sibling, 1 reply; 6+ messages in thread
From: Alex Williamson @ 2025-06-24 20:06 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Ankit Agrawal, Brett Creeley, Giovanni Cabiddu, Kevin Tian, kvm,
	Longfang Liu, qat-linux, virtualization, Xin Zeng, Yishai Hadas,
	Matthew Rosato, Nicolin Chen, patches, Shameer Kolothum,
	Terrence Xu, Yanting Jiang, Yi Liu, Zhenzhong Duan

On Tue, 24 Jun 2025 15:34:40 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> This was missed during the initial implementation. The VFIO PCI encodes
> the vf_token inside the device name when opening the device from the group
> FD, something like:
> 
>   "0000:04:10.0 vf_token=bd8d9d2b-5a5f-4f5a-a211-f591514ba1f3"
> 
> 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/

> VFIO_DEVICE_BIND_IOMMUFD with an optional field indicated by
> VFIO_DEVICE_BIND_TOKEN. Only users using a PCI SRIOV VF will need to
> provide this. This is done in the usual backwards compatible way.
> 
> Fixes: 5fcc26969a16 ("vfio: Add VFIO_DEVICE_BIND_IOMMUFD")
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/vfio/device_cdev.c                    | 38 +++++++++++++++++--
>  .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c    |  1 +
>  drivers/vfio/pci/mlx5/main.c                  |  1 +
>  drivers/vfio/pci/nvgrace-gpu/main.c           |  2 +
>  drivers/vfio/pci/pds/vfio_dev.c               |  1 +
>  drivers/vfio/pci/qat/main.c                   |  1 +
>  drivers/vfio/pci/vfio_pci.c                   |  1 +
>  drivers/vfio/pci/vfio_pci_core.c              | 22 +++++++----
>  drivers/vfio/pci/virtio/main.c                |  3 ++
>  include/linux/vfio.h                          |  1 +
>  include/linux/vfio_pci_core.h                 |  2 +
>  include/uapi/linux/vfio.h                     | 13 ++++++-
>  12 files changed, 74 insertions(+), 12 deletions(-)
> 
> I wrote this quickly and don't have an environment available to check it out
> on.. Since it feels like a significant miss I felt we should start looking at
> it.
> 
> diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
> index 281a8dc3ed4974..c5d74c7e71f585 100644
> --- a/drivers/vfio/device_cdev.c
> +++ b/drivers/vfio/device_cdev.c
> @@ -60,22 +60,50 @@ static void vfio_df_get_kvm_safe(struct vfio_device_file *df)
>  	spin_unlock(&df->kvm_ref_lock);
>  }
>  
> +static int vfio_df_check_token(struct vfio_device *device,
> +			       const struct vfio_device_bind_iommufd *bind)
> +{
> +	uuid_t uuid;
> +
> +	if (!device->ops->match_token_uuid) {
> +		if (bind->flags & VFIO_DEVICE_BIND_TOKEN)
> +			return -EINVAL;
> +		return 0;
> +	}
> +
> +	if (!(bind->flags & VFIO_DEVICE_BIND_TOKEN))
> +		return device->ops->match_token_uuid(device, NULL);
> +
> +	if (copy_from_user(&uuid, u64_to_user_ptr(bind->token_uuid_ptr),
> +			   sizeof(uuid)))
> +		return -EFAULT;
> +	return device->ops->match_token_uuid(device, &uuid);
> +}
> +
>  long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
>  				struct vfio_device_bind_iommufd __user *arg)
>  {
> +	const u32 VALID_FLAGS = VFIO_DEVICE_BIND_TOKEN;
>  	struct vfio_device *device = df->device;
>  	struct vfio_device_bind_iommufd bind;
>  	unsigned long minsz;
> +	u32 user_size;
>  	int ret;
>  
>  	static_assert(__same_type(arg->out_devid, df->devid));
>  
>  	minsz = offsetofend(struct vfio_device_bind_iommufd, out_devid);
>  
> -	if (copy_from_user(&bind, arg, minsz))
> -		return -EFAULT;
> +	ret = get_user(user_size, &arg->argsz);
> +	if (ret)
> +		return ret;
> +	if (bind.argsz < minsz)
> +		return -EINVAL;
> +	ret = copy_struct_from_user(&bind, minsz, arg, user_size);
> +	if (ret)
> +		return ret;
>  
> -	if (bind.argsz < minsz || bind.flags || bind.iommufd < 0)
> +	if (bind.iommufd < 0 || bind.flags & ~VALID_FLAGS)
>  		return -EINVAL;
>  
>  	/* BIND_IOMMUFD only allowed for cdev fds */
> @@ -93,6 +121,10 @@ long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
>  		goto out_unlock;
>  	}
>  
> +	ret = vfio_df_check_token(device, &bind);
> +	if (ret)
> +		return ret;
> +
>  	df->iommufd = iommufd_ctx_from_fd(bind.iommufd);
>  	if (IS_ERR(df->iommufd)) {
>  		ret = PTR_ERR(df->iommufd);
> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> index 2149f49aeec7f8..397f5e44513639 100644
> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> @@ -1583,6 +1583,7 @@ static const struct vfio_device_ops hisi_acc_vfio_pci_ops = {
>  	.mmap = vfio_pci_core_mmap,
>  	.request = vfio_pci_core_request,
>  	.match = vfio_pci_core_match,
> +	.match_token_uuid = vfio_pci_core_match_token_uuid,
>  	.bind_iommufd = vfio_iommufd_physical_bind,
>  	.unbind_iommufd = vfio_iommufd_physical_unbind,
>  	.attach_ioas = vfio_iommufd_physical_attach_ioas,
> diff --git a/drivers/vfio/pci/mlx5/main.c b/drivers/vfio/pci/mlx5/main.c
> index 93f894fe60d221..7ec47e736a8e5a 100644
> --- a/drivers/vfio/pci/mlx5/main.c
> +++ b/drivers/vfio/pci/mlx5/main.c
> @@ -1372,6 +1372,7 @@ static const struct vfio_device_ops mlx5vf_pci_ops = {
>  	.mmap = vfio_pci_core_mmap,
>  	.request = vfio_pci_core_request,
>  	.match = vfio_pci_core_match,
> +	.match_token_uuid = vfio_pci_core_match_token_uuid,
>  	.bind_iommufd = vfio_iommufd_physical_bind,
>  	.unbind_iommufd = vfio_iommufd_physical_unbind,
>  	.attach_ioas = vfio_iommufd_physical_attach_ioas,
> diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c
> index e5ac39c4cc6b6f..d95761dcdd58c4 100644
> --- a/drivers/vfio/pci/nvgrace-gpu/main.c
> +++ b/drivers/vfio/pci/nvgrace-gpu/main.c
> @@ -696,6 +696,7 @@ static const struct vfio_device_ops nvgrace_gpu_pci_ops = {
>  	.mmap		= nvgrace_gpu_mmap,
>  	.request	= vfio_pci_core_request,
>  	.match		= vfio_pci_core_match,
> +	.match_token_uuid = vfio_pci_core_match_token_uuid,
>  	.bind_iommufd	= vfio_iommufd_physical_bind,
>  	.unbind_iommufd	= vfio_iommufd_physical_unbind,
>  	.attach_ioas	= vfio_iommufd_physical_attach_ioas,
> @@ -715,6 +716,7 @@ static const struct vfio_device_ops nvgrace_gpu_pci_core_ops = {
>  	.mmap		= vfio_pci_core_mmap,
>  	.request	= vfio_pci_core_request,
>  	.match		= vfio_pci_core_match,
> +	.match_token_uuid = vfio_pci_core_match_token_uuid,
>  	.bind_iommufd	= vfio_iommufd_physical_bind,
>  	.unbind_iommufd	= vfio_iommufd_physical_unbind,
>  	.attach_ioas	= vfio_iommufd_physical_attach_ioas,
> diff --git a/drivers/vfio/pci/pds/vfio_dev.c b/drivers/vfio/pci/pds/vfio_dev.c
> index 76a80ae7087b51..5731e6856deaf1 100644
> --- a/drivers/vfio/pci/pds/vfio_dev.c
> +++ b/drivers/vfio/pci/pds/vfio_dev.c
> @@ -201,6 +201,7 @@ static const struct vfio_device_ops pds_vfio_ops = {
>  	.mmap = vfio_pci_core_mmap,
>  	.request = vfio_pci_core_request,
>  	.match = vfio_pci_core_match,
> +	.match_token_uuid = vfio_pci_core_match_token_uuid,
>  	.bind_iommufd = vfio_iommufd_physical_bind,
>  	.unbind_iommufd = vfio_iommufd_physical_unbind,
>  	.attach_ioas = vfio_iommufd_physical_attach_ioas,
> diff --git a/drivers/vfio/pci/qat/main.c b/drivers/vfio/pci/qat/main.c
> index 845ed15b67718c..5cce6b0b8d2f3e 100644
> --- a/drivers/vfio/pci/qat/main.c
> +++ b/drivers/vfio/pci/qat/main.c
> @@ -614,6 +614,7 @@ static const struct vfio_device_ops qat_vf_pci_ops = {
>  	.mmap = vfio_pci_core_mmap,
>  	.request = vfio_pci_core_request,
>  	.match = vfio_pci_core_match,
> +	.match_token_uuid = vfio_pci_core_match_token_uuid,
>  	.bind_iommufd = vfio_iommufd_physical_bind,
>  	.unbind_iommufd = vfio_iommufd_physical_unbind,
>  	.attach_ioas = vfio_iommufd_physical_attach_ioas,
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 5ba39f7623bb76..ac10f14417f2f3 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -138,6 +138,7 @@ static const struct vfio_device_ops vfio_pci_ops = {
>  	.mmap		= vfio_pci_core_mmap,
>  	.request	= vfio_pci_core_request,
>  	.match		= vfio_pci_core_match,
> +	.match_token_uuid = vfio_pci_core_match_token_uuid,
>  	.bind_iommufd	= vfio_iommufd_physical_bind,
>  	.unbind_iommufd	= vfio_iommufd_physical_unbind,
>  	.attach_ioas	= vfio_iommufd_physical_attach_ioas,
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 6328c3a05bcdd4..086aa37a60a2b5 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1821,9 +1821,13 @@ void vfio_pci_core_request(struct vfio_device *core_vdev, unsigned int count)
>  }
>  EXPORT_SYMBOL_GPL(vfio_pci_core_request);
>  
> -static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
> -				      bool vf_token, uuid_t *uuid)
> +int vfio_pci_core_match_token_uuid(struct vfio_device *core_vdev,
> +				   const uuid_t *uuid)
> +
>  {
> +	struct vfio_pci_core_device *vdev =
> +		container_of(core_vdev, struct vfio_pci_core_device, vdev);
> +
>  	/*
>  	 * There's always some degree of trust or collaboration between SR-IOV
>  	 * PF and VFs, even if just that the PF hosts the SR-IOV capability and
> @@ -1854,7 +1858,7 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
>  		bool match;
>  
>  		if (!pf_vdev) {
> -			if (!vf_token)
> +			if (!uuid)
>  				return 0; /* PF is not vfio-pci, no VF token */
>  
>  			pci_info_ratelimited(vdev->pdev,
> @@ -1862,7 +1866,7 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
>  			return -EINVAL;
>  		}
>  
> -		if (!vf_token) {
> +		if (!uuid) {
>  			pci_info_ratelimited(vdev->pdev,
>  				"VF token required to access device\n");
>  			return -EACCES;
> @@ -1880,7 +1884,7 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
>  	} else if (vdev->vf_token) {
>  		mutex_lock(&vdev->vf_token->lock);
>  		if (vdev->vf_token->users) {
> -			if (!vf_token) {
> +			if (!uuid) {
>  				mutex_unlock(&vdev->vf_token->lock);
>  				pci_info_ratelimited(vdev->pdev,
>  					"VF token required to access device\n");
> @@ -1893,12 +1897,12 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
>  					"Incorrect VF token provided for device\n");
>  				return -EACCES;
>  			}
> -		} else if (vf_token) {
> +		} else if (uuid) {
>  			uuid_copy(&vdev->vf_token->uuid, uuid);
>  		}
>  
>  		mutex_unlock(&vdev->vf_token->lock);
> -	} else if (vf_token) {
> +	} else if (uuid) {
>  		pci_info_ratelimited(vdev->pdev,
>  			"VF token incorrectly provided, not a PF or VF\n");
>  		return -EINVAL;
> @@ -1906,6 +1910,7 @@ static int vfio_pci_validate_vf_token(struct vfio_pci_core_device *vdev,
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(vfio_pci_core_match_token_uuid);
>  
>  #define VF_TOKEN_ARG "vf_token="
>  
> @@ -1952,7 +1957,8 @@ int vfio_pci_core_match(struct vfio_device *core_vdev, char *buf)
>  		}
>  	}
>  
> -	ret = vfio_pci_validate_vf_token(vdev, vf_token, &uuid);
> +	ret = vfio_pci_core_match_token_uuid(core_vdev,
> +					     vf_token ? &uuid : NULL);

For consistency shouldn't this call core_vdev->ops->match_token_uuid?

>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/vfio/pci/virtio/main.c b/drivers/vfio/pci/virtio/main.c
> index 515fe1b9f94d80..8084f3e36a9f70 100644
> --- a/drivers/vfio/pci/virtio/main.c
> +++ b/drivers/vfio/pci/virtio/main.c
> @@ -94,6 +94,7 @@ static const struct vfio_device_ops virtiovf_vfio_pci_lm_ops = {
>  	.mmap = vfio_pci_core_mmap,
>  	.request = vfio_pci_core_request,
>  	.match = vfio_pci_core_match,
> +	.match_token_uuid = vfio_pci_core_match_token_uuid,
>  	.bind_iommufd = vfio_iommufd_physical_bind,
>  	.unbind_iommufd = vfio_iommufd_physical_unbind,
>  	.attach_ioas = vfio_iommufd_physical_attach_ioas,
> @@ -114,6 +115,7 @@ static const struct vfio_device_ops virtiovf_vfio_pci_tran_lm_ops = {
>  	.mmap = vfio_pci_core_mmap,
>  	.request = vfio_pci_core_request,
>  	.match = vfio_pci_core_match,
> +	.match_token_uuid = vfio_pci_core_match_token_uuid,
>  	.bind_iommufd = vfio_iommufd_physical_bind,
>  	.unbind_iommufd = vfio_iommufd_physical_unbind,
>  	.attach_ioas = vfio_iommufd_physical_attach_ioas,
> @@ -134,6 +136,7 @@ static const struct vfio_device_ops virtiovf_vfio_pci_ops = {
>  	.mmap = vfio_pci_core_mmap,
>  	.request = vfio_pci_core_request,
>  	.match = vfio_pci_core_match,
> +	.match_token_uuid = vfio_pci_core_match_token_uuid,
>  	.bind_iommufd = vfio_iommufd_physical_bind,
>  	.unbind_iommufd = vfio_iommufd_physical_unbind,
>  	.attach_ioas = vfio_iommufd_physical_attach_ioas,
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 707b00772ce1ff..b2cca540a6b4bf 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -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.

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

> +
>  /*
>   * VFIO_DEVICE_BIND_IOMMUFD - _IOR(VFIO_TYPE, VFIO_BASE + 18,
>   *				   struct vfio_device_bind_iommufd)
>   * @argsz:	 User filled size of this data.
> - * @flags:	 Must be 0.
> + * @flags:	 Must be 0 or a bit flags of VFIO_DEVICE_BIND_*
>   * @iommufd:	 iommufd to bind.
>   * @out_devid:	 The device id generated by this bind. devid is a handle for
>   *		 this device/iommufd bond and can be used in IOMMUFD commands.
> + * @token_uuid_ptr: Valid if VFIO_DEVICE_BIND_TOKEN. Points to a 16 byte UUID
> + *                  in the same format as VFIO_DEVICE_FEATURE_PCI_VF_TOKEN.
>   *
>   * Bind a vfio_device to the specified iommufd.
>   *
> @@ -917,6 +921,12 @@ struct vfio_device_feature {
>   *
>   * Unbind is automatically conducted when device fd is closed.
>   *
> + * A token is sometimes required to open the device, unless this is known to be
> + * needed VFIO_DEVICE_BIND_TOKEN should not be set and token_uuid_ptr is
> + * ignored. The only case today is a PF/VF relationship where the VF bind must
> + * be provided the same token as VFIO_DEVICE_FEATURE_PCI_VF_TOKEN provided to
> + * the PF.
> + *
>   * Return: 0 on success, -errno on failure.
>   */
>  struct vfio_device_bind_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.  There's potentially a narrow
case of old code that should have required a token, which now
intentionally breaks.  We're not offering an introspection mechanism
here, but doing so also doesn't add a lot of value.  Userspace needs to
know the token to pass anyway.  Is that how you see it?

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.

Thanks for jumping on this,
Alex

>  
>  #define VFIO_DEVICE_BIND_IOMMUFD	_IO(VFIO_TYPE, VFIO_BASE + 18)
> 
> base-commit: 3e2a9811f6a9cefd310cc33cab73d5435b4a4caa


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

* Re: [PATCH] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-06-24 20:06 ` Alex Williamson
@ 2025-07-02 14:56   ` Jason Gunthorpe
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Gunthorpe @ 2025-07-02 14:56 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Ankit Agrawal, Brett Creeley, Giovanni Cabiddu, Kevin Tian, kvm,
	Longfang Liu, qat-linux, virtualization, Xin Zeng, Yishai Hadas,
	Matthew Rosato, Nicolin Chen, patches, Shameer Kolothum,
	Terrence Xu, Yanting Jiang, Yi Liu, Zhenzhong Duan

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

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

* RE: [PATCH] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-06-24 18:34 [PATCH] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD Jason Gunthorpe
  2025-06-24 20:06 ` Alex Williamson
@ 2025-07-03  6:40 ` Tian, Kevin
  2025-07-03 13:41   ` Jason Gunthorpe
  1 sibling, 1 reply; 6+ messages in thread
From: Tian, Kevin @ 2025-07-03  6:40 UTC (permalink / raw)
  To: Jason Gunthorpe, Ankit Agrawal, Brett Creeley, Cabiddu, Giovanni,
	kvm@vger.kernel.org, Longfang Liu, qat-linux,
	virtualization@lists.linux.dev, Zeng, Xin, Yishai Hadas
  Cc: Alex Williamson, Matthew Rosato, Nicolin Chen,
	patches@lists.linux.dev, Shameer Kolothum, Xu, Terrence,
	Jiang, Yanting, Liu, Yi L, Duan, Zhenzhong

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, June 25, 2025 2:35 AM
> 
> This was missed during the initial implementation. The VFIO PCI encodes
> the vf_token inside the device name when opening the device from the
> group
> FD, something like:
> 
>   "0000:04:10.0 vf_token=bd8d9d2b-5a5f-4f5a-a211-f591514ba1f3"
> 
> 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
> VFIO_DEVICE_BIND_IOMMUFD with an optional field indicated by
> VFIO_DEVICE_BIND_TOKEN.

not a complete sentence?

> Only users using a PCI SRIOV VF will need to
> provide this. This is done in the usual backwards compatible way.

and PF also needs to provide it when there are in-use VFs:

vfio_pci_validate_vf_token():
         * When presented with a PF which has VFs in use, the user must also
         * provide the current VF token to prove collaboration with existing
         * VF users.  If VFs are not in use, the VF token provided for the PF
         * device will act to set the VF token.

> @@ -1583,6 +1583,7 @@ static const struct vfio_device_ops
> hisi_acc_vfio_pci_ops = {
>  	.mmap = vfio_pci_core_mmap,
>  	.request = vfio_pci_core_request,
>  	.match = vfio_pci_core_match,
> +	.match_token_uuid = vfio_pci_core_match_token_uuid,

this matters only when the driver supports SR-IOV. currently only
vfio-pci does.

what about adding a check of it with .sriov_configure() in
vfio_pci_core_register_device() to save changes in every driver?

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

* Re: [PATCH] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-07-03  6:40 ` Tian, Kevin
@ 2025-07-03 13:41   ` Jason Gunthorpe
  2025-07-10  8:29     ` Tian, Kevin
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Gunthorpe @ 2025-07-03 13:41 UTC (permalink / raw)
  To: Tian, Kevin
  Cc: Ankit Agrawal, Brett Creeley, Cabiddu, Giovanni,
	kvm@vger.kernel.org, Longfang Liu, qat-linux,
	virtualization@lists.linux.dev, Zeng, Xin, Yishai Hadas,
	Alex Williamson, Matthew Rosato, Nicolin Chen,
	patches@lists.linux.dev, Shameer Kolothum, Xu, Terrence,
	Jiang, Yanting, Liu, Yi L, Duan, Zhenzhong

On Thu, Jul 03, 2025 at 06:40:48AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Wednesday, June 25, 2025 2:35 AM
> > 
> > This was missed during the initial implementation. The VFIO PCI encodes
> > the vf_token inside the device name when opening the device from the
> > group
> > FD, something like:
> > 
> >   "0000:04:10.0 vf_token=bd8d9d2b-5a5f-4f5a-a211-f591514ba1f3"
> > 
> > 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
> > VFIO_DEVICE_BIND_IOMMUFD with an optional field indicated by
> > VFIO_DEVICE_BIND_TOKEN.
> 
> not a complete sentence?

 Since we no longer have a device name, pass the token directly through
 VFIO_DEVICE_BIND_IOMMUFD using an optional field indicated by
 VFIO_DEVICE_BIND_TOKEN.


> > Only users using a PCI SRIOV VF will need to
> > provide this. This is done in the usual backwards compatible way.
> 
> and PF also needs to provide it when there are in-use VFs:
> 
> vfio_pci_validate_vf_token():
>          * When presented with a PF which has VFs in use, the user must also
>          * provide the current VF token to prove collaboration with existing
>          * VF users.  If VFs are not in use, the VF token provided for the PF
>          * device will act to set the VF token.

Too complicated, I'll just drop that sentence.

> > @@ -1583,6 +1583,7 @@ static const struct vfio_device_ops
> > hisi_acc_vfio_pci_ops = {
> >  	.mmap = vfio_pci_core_mmap,
> >  	.request = vfio_pci_core_request,
> >  	.match = vfio_pci_core_match,
> > +	.match_token_uuid = vfio_pci_core_match_token_uuid,
> 
> this matters only when the driver supports SR-IOV. currently only
> vfio-pci does.

Hmm, sriov_pf_core_dev requires sriov_config, but the normal vf_token
happens for all vfs and there is a little debugging related to it:

			pci_info_ratelimited(vdev->pdev,
				"VF token incorrectly provided, PF not bound to vfio-pci\n");

So maybe we want to keep it. Otherwise it sounds like you are
proposing to remove match from most of the drivers since they don't
support sriov_configure? Which ever direction I think match and
match_token_uuid should be together.

Jason

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

* RE: [PATCH] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-07-03 13:41   ` Jason Gunthorpe
@ 2025-07-10  8:29     ` Tian, Kevin
  0 siblings, 0 replies; 6+ messages in thread
From: Tian, Kevin @ 2025-07-10  8:29 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Ankit Agrawal, Brett Creeley, Cabiddu, Giovanni,
	kvm@vger.kernel.org, Longfang Liu, qat-linux,
	virtualization@lists.linux.dev, Zeng, Xin, Yishai Hadas,
	Alex Williamson, Matthew Rosato, Nicolin Chen,
	patches@lists.linux.dev, Shameer Kolothum, Xu, Terrence,
	Jiang, Yanting, Liu, Yi L, Duan, Zhenzhong

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, July 3, 2025 9:41 PM
> 
> On Thu, Jul 03, 2025 at 06:40:48AM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > Sent: Wednesday, June 25, 2025 2:35 AM
> > >
> > > @@ -1583,6 +1583,7 @@ static const struct vfio_device_ops
> > > hisi_acc_vfio_pci_ops = {
> > >  	.mmap = vfio_pci_core_mmap,
> > >  	.request = vfio_pci_core_request,
> > >  	.match = vfio_pci_core_match,
> > > +	.match_token_uuid = vfio_pci_core_match_token_uuid,
> >
> > this matters only when the driver supports SR-IOV. currently only
> > vfio-pci does.
> 
> Hmm, sriov_pf_core_dev requires sriov_config, but the normal vf_token
> happens for all vfs and there is a little debugging related to it:
> 
> 			pci_info_ratelimited(vdev->pdev,
> 				"VF token incorrectly provided, PF not bound
> to vfio-pci\n");
> 
> So maybe we want to keep it. Otherwise it sounds like you are
> proposing to remove match from most of the drivers since they don't
> support sriov_configure? Which ever direction I think match and
> match_token_uuid should be together.
> 

Okay that makes sense.

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

end of thread, other threads:[~2025-07-10  8:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-24 18:34 [PATCH] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD Jason Gunthorpe
2025-06-24 20:06 ` Alex Williamson
2025-07-02 14:56   ` Jason Gunthorpe
2025-07-03  6:40 ` Tian, Kevin
2025-07-03 13:41   ` Jason Gunthorpe
2025-07-10  8:29     ` Tian, Kevin

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