Re: [PATCH v4 1/4] iommufd/device: Associate a kvm pointer to iommufd_device

[Baolu Lu]

On 4/27/2026 2:10 PM, Aneesh Kumar K.V (Arm) wrote:
> From: Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx>
>
> Add a struct kvm * to iommufd_device_bind() fn and associate it
> with idev if bind is successful.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx>
> Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> [nicolinc: fix build error in iommufd_test_mock_domain()]
> Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@xxxxxxxxxx>
> ---
>   drivers/iommu/iommufd/device.c          | 5 ++++-
>   drivers/iommu/iommufd/iommufd_private.h | 2 ++
>   drivers/iommu/iommufd/selftest.c        | 2 +-
>   drivers/vfio/iommufd.c                  | 2 +-
>   include/linux/iommufd.h                 | 4 +++-
>   5 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index 344d620cdecc..453fbceb9219 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -203,6 +203,7 @@ void iommufd_device_destroy(struct iommufd_object *obj)
>    * iommufd_device_bind - Bind a physical device to an iommu fd
>    * @ictx: iommufd file descriptor
>    * @dev: Pointer to a physical device struct
> + * @kvm: Pointer to struct kvm if device belongs to a KVM VM
>    * @id: Output ID number to return to userspace for this device
>    *
>    * A successful bind establishes an ownership over the device and returns
> @@ -216,7 +217,8 @@ void iommufd_device_destroy(struct iommufd_object *obj)
>    * The caller must undo this with iommufd_device_unbind()
>    */
>   struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
> -					   struct device *dev, u32 *id)
> +					   struct device *dev, struct kvm *kvm,
> +					   u32 *id)
>   {
>   	struct iommufd_device *idev;
>   	struct iommufd_group *igroup;
> @@ -266,6 +268,7 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
>   	if (!iommufd_selftest_is_mock_dev(dev))
>   		iommufd_ctx_get(ictx);
>   	idev->dev = dev;
> +	idev->kvm = kvm;
>   	idev->enforce_cache_coherency =
>   		device_iommu_capable(dev, IOMMU_CAP_ENFORCE_CACHE_COHERENCY);
>   	/* The calling driver is a user until iommufd_device_unbind() */
> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
> index 6ac1965199e9..c48a568c6cbb 100644
> --- a/drivers/iommu/iommufd/iommufd_private.h
> +++ b/drivers/iommu/iommufd/iommufd_private.h
> @@ -488,6 +488,8 @@ struct iommufd_device {
>   	struct list_head group_item;
>   	/* always the physical device */
>   	struct device *dev;
> +	/* ..and kvm if available */
> +	struct kvm *kvm;
>   	bool enforce_cache_coherency;
>   	struct iommufd_vdevice *vdev;
>   	bool destroying;
> diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
> index 7823142097d4..76a2f83f430c 100644
> --- a/drivers/iommu/iommufd/selftest.c
> +++ b/drivers/iommu/iommufd/selftest.c
> @@ -1100,7 +1100,7 @@ static int iommufd_test_mock_domain(struct iommufd_ucmd *ucmd,
>   		goto out_sobj;
>   	}
>   
> -	idev = iommufd_device_bind(ucmd->ictx, &sobj->idev.mock_dev->dev,
> +	idev = iommufd_device_bind(ucmd->ictx, &sobj->idev.mock_dev->dev, NULL,
>   				   &idev_id);
>   	if (IS_ERR(idev)) {
>   		rc = PTR_ERR(idev);
> diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c
> index a38d262c6028..c1c58194fd3a 100644
> --- a/drivers/vfio/iommufd.c
> +++ b/drivers/vfio/iommufd.c
> @@ -119,7 +119,7 @@ int vfio_iommufd_physical_bind(struct vfio_device *vdev,
>   {
>   	struct iommufd_device *idev;
>   
> -	idev = iommufd_device_bind(ictx, vdev->dev, out_device_id);
> +	idev = iommufd_device_bind(ictx, vdev->dev, vdev->kvm, out_device_id);
>   	if (IS_ERR(idev))
>   		return PTR_ERR(idev);
>   	vdev->iommufd_device = idev;
> diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
> index 6e7efe83bc5d..5cdcb8d2f305 100644
> --- a/include/linux/iommufd.h
> +++ b/include/linux/iommufd.h
> @@ -24,6 +24,7 @@ struct iommufd_ctx;
>   struct iommufd_device;
>   struct iommufd_viommu_ops;
>   struct page;
> +struct kvm;
>   
>   enum iommufd_object_type {
>   	IOMMUFD_OBJ_NONE,
> @@ -59,7 +60,8 @@ struct iommufd_object {
>   };
>   
>   struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
> -					   struct device *dev, u32 *id);
> +					   struct device *dev, struct kvm *kvm,
> +					   u32 *id);
>   void iommufd_device_unbind(struct iommufd_device *idev);
>   
>   int iommufd_device_attach(struct iommufd_device *idev, ioasid_t pasid,

iommufd_device_unbind() is an asynchronous teardown operation. Is it
possible for idev->kvm to persist or be accessed within the iommufd
subsystem after iommufd_device_unbind() has returned? Should we add the
change below to prevent this potential UAF case?

diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 170a7005f0bc..dac39cb5e7cc 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -333,6 +333,7 @@ EXPORT_SYMBOL_NS_GPL(iommufd_ctx_has_group, "IOMMUFD");
  */
 void iommufd_device_unbind(struct iommufd_device *idev)
 {
+       idev->kvm = NULL;
        iommufd_object_destroy_user(idev->ictx, &idev->obj);
 }
 EXPORT_SYMBOL_NS_GPL(iommufd_device_unbind, "IOMMUFD");

Otherwise, it looks good to me.

Reviewed-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>