kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
@ 2025-07-14 16:08 Jason Gunthorpe
  2025-07-15  0:00 ` Tian, Kevin
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jason Gunthorpe @ 2025-07-14 16:08 UTC (permalink / raw)
  To: Alex Williamson, Ankit Agrawal, Brett Creeley, Giovanni Cabiddu,
	Kevin Tian, kvm, Longfang Liu, qat-linux, Shameer Kolothum,
	virtualization, Xin Zeng, Yishai Hadas
  Cc: patches

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 in the cdev path, pass the token
directly through VFIO_DEVICE_BIND_IOMMUFD using an optional field
indicated by VFIO_DEVICE_BIND_FLAG_TOKEN.

Fixes: 5fcc26969a16 ("vfio: Add VFIO_DEVICE_BIND_IOMMUFD")
Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Yi Liu <yi.l.liu@intel.com>
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                          |  4 ++
 include/linux/vfio_pci_core.h                 |  2 +
 include/uapi/linux/vfio.h                     | 12 +++++-
 12 files changed, 76 insertions(+), 12 deletions(-)

v3:
 - Fix user_size check
v2: https://lore.kernel.org/all/0-v2-470f044801ef+a887e-vfio_token_jgg@nvidia.com
 - Revise VFIO_DEVICE_BIND_TOKEN -> VFIO_DEVICE_BIND_FLAG_TOKEN
 - Call the match_token_uuid through ops instead of directly
 - update comments/style
v1: https://patch.msgid.link/r/0-v1-8639f9aed215+853-vfio_token_jgg@nvidia.com

diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
index 281a8dc3ed4974..53a602563f002d 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_FLAG_TOKEN)
+			return -EINVAL;
+		return 0;
+	}
+
+	if (!(bind->flags & VFIO_DEVICE_BIND_FLAG_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_FLAG_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 (user_size < 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..d39b0201d910fd 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 = core_vdev->ops->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..eb563f538dee51 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -105,6 +105,9 @@ struct vfio_device {
  * @match: Optional device name match callback (return: 0 for no-match, >0 for
  *         match, -errno for abort (ex. match with insufficient or incorrect
  *         additional args)
+ * @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.
  * @dma_unmap: Called when userspace unmaps IOVA from the container
  *             this device is attached to.
  * @device_feature: Optional, fill in the VFIO_DEVICE_FEATURE ioctl
@@ -132,6 +135,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..75100bf009baf5 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -905,10 +905,12 @@ struct vfio_device_feature {
  * 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_FLAG_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,13 +919,21 @@ 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_FLAG_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 {
 	__u32		argsz;
 	__u32		flags;
+#define VFIO_DEVICE_BIND_FLAG_TOKEN (1 << 0)
 	__s32		iommufd;
 	__u32		out_devid;
+	__aligned_u64	token_uuid_ptr;
 };
 
 #define VFIO_DEVICE_BIND_IOMMUFD	_IO(VFIO_TYPE, VFIO_BASE + 18)

base-commit: 32b2d3a57e26804ca96d82a222667ac0fa226cb7
-- 
2.43.0


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

* RE: [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-07-14 16:08 [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD Jason Gunthorpe
@ 2025-07-15  0:00 ` Tian, Kevin
  2025-07-15 22:55 ` Dan Carpenter
  2025-07-16 20:22 ` Alex Williamson
  2 siblings, 0 replies; 6+ messages in thread
From: Tian, Kevin @ 2025-07-15  0:00 UTC (permalink / raw)
  To: Jason Gunthorpe, Alex Williamson, Ankit Agrawal, Brett Creeley,
	Cabiddu, Giovanni, kvm@vger.kernel.org, Longfang Liu, qat-linux,
	Shameer Kolothum, virtualization@lists.linux.dev, Zeng, Xin,
	Yishai Hadas
  Cc: patches@lists.linux.dev

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Tuesday, July 15, 2025 12:08 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 in the cdev path, pass the token
> directly through VFIO_DEVICE_BIND_IOMMUFD using an optional field
> indicated by VFIO_DEVICE_BIND_FLAG_TOKEN.
> 
> Fixes: 5fcc26969a16 ("vfio: Add VFIO_DEVICE_BIND_IOMMUFD")
> Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

my r-b to v2 was missed. anyway:

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

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

* Re: [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-07-14 16:08 [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD Jason Gunthorpe
  2025-07-15  0:00 ` Tian, Kevin
@ 2025-07-15 22:55 ` Dan Carpenter
  2025-07-15 23:06   ` Jason Gunthorpe
  2025-07-16 20:22 ` Alex Williamson
  2 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2025-07-15 22:55 UTC (permalink / raw)
  To: oe-kbuild, Jason Gunthorpe, Alex Williamson, Ankit Agrawal,
	Brett Creeley, Giovanni Cabiddu, Kevin Tian, kvm, Longfang Liu,
	qat-linux, Shameer Kolothum, virtualization, Xin Zeng,
	Yishai Hadas
  Cc: lkp, oe-kbuild-all, patches

Hi Jason,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Jason-Gunthorpe/vfio-pci-Do-vf_token-checks-for-VFIO_DEVICE_BIND_IOMMUFD/20250715-001209
base:   32b2d3a57e26804ca96d82a222667ac0fa226cb7
patch link:    https://lore.kernel.org/r/0-v3-bdd8716e85fe%2B3978a-vfio_token_jgg%40nvidia.com
patch subject: [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
config: openrisc-randconfig-r071-20250715 (https://download.01.org/0day-ci/archive/20250716/202507160254.dAjYAz9h-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 15.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202507160254.dAjYAz9h-lkp@intel.com/

smatch warnings:
drivers/vfio/device_cdev.c:126 vfio_df_ioctl_bind_iommufd() warn: missing unwind goto?
drivers/vfio/device_cdev.c:170 vfio_df_ioctl_bind_iommufd() warn: inconsistent returns '&device->dev_set->lock'.

vim +126 drivers/vfio/device_cdev.c

5fcc26969a164e Yi Liu          2023-07-18   83  long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
5fcc26969a164e Yi Liu          2023-07-18   84  				struct vfio_device_bind_iommufd __user *arg)
5fcc26969a164e Yi Liu          2023-07-18   85  {
be2e70b96c3e54 Jason Gunthorpe 2025-07-14   86  	const u32 VALID_FLAGS = VFIO_DEVICE_BIND_FLAG_TOKEN;
5fcc26969a164e Yi Liu          2023-07-18   87  	struct vfio_device *device = df->device;
5fcc26969a164e Yi Liu          2023-07-18   88  	struct vfio_device_bind_iommufd bind;
5fcc26969a164e Yi Liu          2023-07-18   89  	unsigned long minsz;
be2e70b96c3e54 Jason Gunthorpe 2025-07-14   90  	u32 user_size;
5fcc26969a164e Yi Liu          2023-07-18   91  	int ret;
5fcc26969a164e Yi Liu          2023-07-18   92  
5fcc26969a164e Yi Liu          2023-07-18   93  	static_assert(__same_type(arg->out_devid, df->devid));
5fcc26969a164e Yi Liu          2023-07-18   94  
5fcc26969a164e Yi Liu          2023-07-18   95  	minsz = offsetofend(struct vfio_device_bind_iommufd, out_devid);
5fcc26969a164e Yi Liu          2023-07-18   96  
be2e70b96c3e54 Jason Gunthorpe 2025-07-14   97  	ret = get_user(user_size, &arg->argsz);
be2e70b96c3e54 Jason Gunthorpe 2025-07-14   98  	if (ret)
be2e70b96c3e54 Jason Gunthorpe 2025-07-14   99  		return ret;
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  100  	if (user_size < minsz)
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  101  		return -EINVAL;
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  102  	ret = copy_struct_from_user(&bind, minsz, arg, user_size);
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  103  	if (ret)
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  104  		return ret;
5fcc26969a164e Yi Liu          2023-07-18  105  
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  106  	if (bind.iommufd < 0 || bind.flags & ~VALID_FLAGS)
5fcc26969a164e Yi Liu          2023-07-18  107  		return -EINVAL;
5fcc26969a164e Yi Liu          2023-07-18  108  
5fcc26969a164e Yi Liu          2023-07-18  109  	/* BIND_IOMMUFD only allowed for cdev fds */
5fcc26969a164e Yi Liu          2023-07-18  110  	if (df->group)
5fcc26969a164e Yi Liu          2023-07-18  111  		return -EINVAL;
5fcc26969a164e Yi Liu          2023-07-18  112  
5fcc26969a164e Yi Liu          2023-07-18  113  	ret = vfio_device_block_group(device);
5fcc26969a164e Yi Liu          2023-07-18  114  	if (ret)
5fcc26969a164e Yi Liu          2023-07-18  115  		return ret;
5fcc26969a164e Yi Liu          2023-07-18  116  
5fcc26969a164e Yi Liu          2023-07-18  117  	mutex_lock(&device->dev_set->lock);
5fcc26969a164e Yi Liu          2023-07-18  118  	/* one device cannot be bound twice */
5fcc26969a164e Yi Liu          2023-07-18  119  	if (df->access_granted) {
5fcc26969a164e Yi Liu          2023-07-18  120  		ret = -EINVAL;
5fcc26969a164e Yi Liu          2023-07-18  121  		goto out_unlock;
5fcc26969a164e Yi Liu          2023-07-18  122  	}
5fcc26969a164e Yi Liu          2023-07-18  123  
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  124  	ret = vfio_df_check_token(device, &bind);
be2e70b96c3e54 Jason Gunthorpe 2025-07-14  125  	if (ret)
be2e70b96c3e54 Jason Gunthorpe 2025-07-14 @126  		return ret;

This needs to be a goto unlock.

be2e70b96c3e54 Jason Gunthorpe 2025-07-14  127  
5fcc26969a164e Yi Liu          2023-07-18  128  	df->iommufd = iommufd_ctx_from_fd(bind.iommufd);
5fcc26969a164e Yi Liu          2023-07-18  129  	if (IS_ERR(df->iommufd)) {
5fcc26969a164e Yi Liu          2023-07-18  130  		ret = PTR_ERR(df->iommufd);
5fcc26969a164e Yi Liu          2023-07-18  131  		df->iommufd = NULL;
5fcc26969a164e Yi Liu          2023-07-18  132  		goto out_unlock;
5fcc26969a164e Yi Liu          2023-07-18  133  	}
5fcc26969a164e Yi Liu          2023-07-18  134  
5fcc26969a164e Yi Liu          2023-07-18  135  	/*
5fcc26969a164e Yi Liu          2023-07-18  136  	 * Before the device open, get the KVM pointer currently
5fcc26969a164e Yi Liu          2023-07-18  137  	 * associated with the device file (if there is) and obtain
5fcc26969a164e Yi Liu          2023-07-18  138  	 * a reference.  This reference is held until device closed.
5fcc26969a164e Yi Liu          2023-07-18  139  	 * Save the pointer in the device for use by drivers.
5fcc26969a164e Yi Liu          2023-07-18  140  	 */
5fcc26969a164e Yi Liu          2023-07-18  141  	vfio_df_get_kvm_safe(df);
5fcc26969a164e Yi Liu          2023-07-18  142  
5fcc26969a164e Yi Liu          2023-07-18  143  	ret = vfio_df_open(df);
5fcc26969a164e Yi Liu          2023-07-18  144  	if (ret)
5fcc26969a164e Yi Liu          2023-07-18  145  		goto out_put_kvm;
5fcc26969a164e Yi Liu          2023-07-18  146  
5fcc26969a164e Yi Liu          2023-07-18  147  	ret = copy_to_user(&arg->out_devid, &df->devid,
5fcc26969a164e Yi Liu          2023-07-18  148  			   sizeof(df->devid)) ? -EFAULT : 0;
5fcc26969a164e Yi Liu          2023-07-18  149  	if (ret)
5fcc26969a164e Yi Liu          2023-07-18  150  		goto out_close_device;
5fcc26969a164e Yi Liu          2023-07-18  151  
5fcc26969a164e Yi Liu          2023-07-18  152  	device->cdev_opened = true;
5fcc26969a164e Yi Liu          2023-07-18  153  	/*
5fcc26969a164e Yi Liu          2023-07-18  154  	 * Paired with smp_load_acquire() in vfio_device_fops::ioctl/
5fcc26969a164e Yi Liu          2023-07-18  155  	 * read/write/mmap
5fcc26969a164e Yi Liu          2023-07-18  156  	 */
5fcc26969a164e Yi Liu          2023-07-18  157  	smp_store_release(&df->access_granted, true);
5fcc26969a164e Yi Liu          2023-07-18  158  	mutex_unlock(&device->dev_set->lock);
5fcc26969a164e Yi Liu          2023-07-18  159  	return 0;
5fcc26969a164e Yi Liu          2023-07-18  160  
5fcc26969a164e Yi Liu          2023-07-18  161  out_close_device:
5fcc26969a164e Yi Liu          2023-07-18  162  	vfio_df_close(df);
5fcc26969a164e Yi Liu          2023-07-18  163  out_put_kvm:
5fcc26969a164e Yi Liu          2023-07-18  164  	vfio_device_put_kvm(device);
5fcc26969a164e Yi Liu          2023-07-18  165  	iommufd_ctx_put(df->iommufd);
5fcc26969a164e Yi Liu          2023-07-18  166  	df->iommufd = NULL;
5fcc26969a164e Yi Liu          2023-07-18  167  out_unlock:
5fcc26969a164e Yi Liu          2023-07-18  168  	mutex_unlock(&device->dev_set->lock);
5fcc26969a164e Yi Liu          2023-07-18  169  	vfio_device_unblock_group(device);
5fcc26969a164e Yi Liu          2023-07-18 @170  	return ret;
5fcc26969a164e Yi Liu          2023-07-18  171  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


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

* Re: [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-07-15 22:55 ` Dan Carpenter
@ 2025-07-15 23:06   ` Jason Gunthorpe
  2025-07-16 17:00     ` Alex Williamson
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Gunthorpe @ 2025-07-15 23:06 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, Alex Williamson, Ankit Agrawal, Brett Creeley,
	Giovanni Cabiddu, Kevin Tian, kvm, Longfang Liu, qat-linux,
	Shameer Kolothum, virtualization, Xin Zeng, Yishai Hadas, lkp,
	oe-kbuild-all, patches

On Wed, Jul 16, 2025 at 01:55:45AM +0300, Dan Carpenter wrote:
> 5fcc26969a164e Yi Liu          2023-07-18  117  	mutex_lock(&device->dev_set->lock);
> 5fcc26969a164e Yi Liu          2023-07-18  118  	/* one device cannot be bound twice */
> 5fcc26969a164e Yi Liu          2023-07-18  119  	if (df->access_granted) {
> 5fcc26969a164e Yi Liu          2023-07-18  120  		ret = -EINVAL;
> 5fcc26969a164e Yi Liu          2023-07-18  121  		goto out_unlock;
> 5fcc26969a164e Yi Liu          2023-07-18  122  	}
> 5fcc26969a164e Yi Liu          2023-07-18  123  
> be2e70b96c3e54 Jason Gunthorpe 2025-07-14  124  	ret = vfio_df_check_token(device, &bind);
> be2e70b96c3e54 Jason Gunthorpe 2025-07-14  125  	if (ret)
> be2e70b96c3e54 Jason Gunthorpe 2025-07-14 @126  		return ret;
> 
> This needs to be a goto unlock.

Oop yes, thank you

Alex can you fix it up when applying?

Thanks,
Jason

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

* Re: [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-07-15 23:06   ` Jason Gunthorpe
@ 2025-07-16 17:00     ` Alex Williamson
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Williamson @ 2025-07-16 17:00 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Dan Carpenter, oe-kbuild, Ankit Agrawal, Brett Creeley,
	Giovanni Cabiddu, Kevin Tian, kvm, Longfang Liu, qat-linux,
	Shameer Kolothum, virtualization, Xin Zeng, Yishai Hadas, lkp,
	oe-kbuild-all, patches

On Tue, 15 Jul 2025 20:06:18 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Wed, Jul 16, 2025 at 01:55:45AM +0300, Dan Carpenter wrote:
> > 5fcc26969a164e Yi Liu          2023-07-18  117  	mutex_lock(&device->dev_set->lock);
> > 5fcc26969a164e Yi Liu          2023-07-18  118  	/* one device cannot be bound twice */
> > 5fcc26969a164e Yi Liu          2023-07-18  119  	if (df->access_granted) {
> > 5fcc26969a164e Yi Liu          2023-07-18  120  		ret = -EINVAL;
> > 5fcc26969a164e Yi Liu          2023-07-18  121  		goto out_unlock;
> > 5fcc26969a164e Yi Liu          2023-07-18  122  	}
> > 5fcc26969a164e Yi Liu          2023-07-18  123  
> > be2e70b96c3e54 Jason Gunthorpe 2025-07-14  124  	ret = vfio_df_check_token(device, &bind);
> > be2e70b96c3e54 Jason Gunthorpe 2025-07-14  125  	if (ret)
> > be2e70b96c3e54 Jason Gunthorpe 2025-07-14 @126  		return ret;
> > 
> > This needs to be a goto unlock.  
> 
> Oop yes, thank you
> 
> Alex can you fix it up when applying?

Yes, I'll apply with:

diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
index 53a602563f00..480cac3a0c27 100644
--- a/drivers/vfio/device_cdev.c
+++ b/drivers/vfio/device_cdev.c
@@ -123,7 +123,7 @@ long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
 
 	ret = vfio_df_check_token(device, &bind);
 	if (ret)
-		return ret;
+		goto out_unlock;
 
 	df->iommufd = iommufd_ctx_from_fd(bind.iommufd);
 	if (IS_ERR(df->iommufd)) {


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

* Re: [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD
  2025-07-14 16:08 [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD Jason Gunthorpe
  2025-07-15  0:00 ` Tian, Kevin
  2025-07-15 22:55 ` Dan Carpenter
@ 2025-07-16 20:22 ` Alex Williamson
  2 siblings, 0 replies; 6+ messages in thread
From: Alex Williamson @ 2025-07-16 20:22 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Ankit Agrawal, Brett Creeley, Giovanni Cabiddu, Kevin Tian, kvm,
	Longfang Liu, qat-linux, Shameer Kolothum, virtualization,
	Xin Zeng, Yishai Hadas, patches

On Mon, 14 Jul 2025 13:08:25 -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 in the cdev path, pass the token
> directly through VFIO_DEVICE_BIND_IOMMUFD using an optional field
> indicated by VFIO_DEVICE_BIND_FLAG_TOKEN.
> 
> Fixes: 5fcc26969a16 ("vfio: Add VFIO_DEVICE_BIND_IOMMUFD")
> Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
> 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                          |  4 ++
>  include/linux/vfio_pci_core.h                 |  2 +
>  include/uapi/linux/vfio.h                     | 12 +++++-
>  12 files changed, 76 insertions(+), 12 deletions(-)

Applied to vfio next branch for v6.17 with discussed fix.  Thanks,

Alex


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

end of thread, other threads:[~2025-07-16 20:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-14 16:08 [PATCH v3] vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD Jason Gunthorpe
2025-07-15  0:00 ` Tian, Kevin
2025-07-15 22:55 ` Dan Carpenter
2025-07-15 23:06   ` Jason Gunthorpe
2025-07-16 17:00     ` Alex Williamson
2025-07-16 20:22 ` Alex Williamson

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