From 8062148046e1a6417d44e2ed86c04e66c2f4f2a1 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Mon, 13 Jul 2026 14:09:46 -0300 Subject: [PATCH] iommufd: Simplify iommufd_device_remove_vdev() Peiyang reports that this function indirectly includes a fault injection point through iommufd_get_object() that was intended to cover the uAPI use of object IDs, not in places like this that cannot fail. On deeper inspection this can be written using a dedicated helper to obtain a users refcount relying entirely on the xa locking instead of going through the whole get/put scheme. The new helper doesn't need the fault injection point. Fixes: 850f14f5b919 ("iommufd: Destroy vdevice on idevice destroy") Link: https://patch.msgid.link/r/0-v1-719003d53a5b+38b-iommufd_fault_inj_vdev_jgg@nvidia.com Reported-by: Peiyang He Closes: https://lore.kernel.org/r/870BB9ADBBEDDD1A+37c5bfab-ad32-4fc5-a302-57c81a8432b5@smail.nju.edu.cn Reviewed-by: Nicolin Chen Reviewed-by: Kevin Tian Signed-off-by: Jason Gunthorpe --- drivers/iommu/iommufd/device.c | 13 +++---------- drivers/iommu/iommufd/iommufd_private.h | 9 +-------- drivers/iommu/iommufd/main.c | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index f1a45a341d46..7deb3346cb78 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -148,29 +148,22 @@ static void iommufd_device_remove_vdev(struct iommufd_device *idev) if (!idev->vdev) goto out_unlock; - vdev = iommufd_get_vdevice(idev->ictx, idev->vdev->obj.id); + vdev = idev->vdev; + /* * An ongoing vdev destroy ioctl has removed the vdev from the object * xarray, but has not finished iommufd_vdevice_destroy() yet as it * needs the same mutex. We exit the locking then wait on wait_cnt * reference for the vdev destruction. */ - if (IS_ERR(vdev)) + if (iommufd_try_inc_users(idev->ictx, &vdev->obj)) goto out_unlock; - /* Should never happen */ - if (WARN_ON(vdev != idev->vdev)) { - iommufd_put_object(idev->ictx, &vdev->obj); - goto out_unlock; - } - /* * vdev is still alive. Hold a users refcount to prevent racing with * userspace destruction, then use iommufd_object_tombstone_user() to * destroy it and leave a tombstone. */ - refcount_inc(&vdev->obj.users); - iommufd_put_object(idev->ictx, &vdev->obj); mutex_unlock(&idev->igroup->lock); iommufd_object_tombstone_user(idev->ictx, &vdev->obj); return; diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 43fbc5bed8de..421d0cc7c1bc 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -182,6 +182,7 @@ static inline bool iommufd_lock_obj(struct iommufd_object *obj) return true; } +int iommufd_try_inc_users(struct iommufd_ctx *ictx, struct iommufd_object *obj); struct iommufd_object *iommufd_get_object(struct iommufd_ctx *ictx, u32 id, enum iommufd_object_type type); static inline void iommufd_put_object(struct iommufd_ctx *ictx, @@ -698,14 +699,6 @@ void iommufd_vdevice_abort(struct iommufd_object *obj); int iommufd_hw_queue_alloc_ioctl(struct iommufd_ucmd *ucmd); void iommufd_hw_queue_destroy(struct iommufd_object *obj); -static inline struct iommufd_vdevice * -iommufd_get_vdevice(struct iommufd_ctx *ictx, u32 id) -{ - return container_of(iommufd_get_object(ictx, id, - IOMMUFD_OBJ_VDEVICE), - struct iommufd_vdevice, obj); -} - #ifdef CONFIG_IOMMUFD_TEST int iommufd_test(struct iommufd_ucmd *ucmd); void iommufd_selftest_destroy(struct iommufd_object *obj); diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c index 8c6d43601afb..e1097a1db21a 100644 --- a/drivers/iommu/iommufd/main.c +++ b/drivers/iommu/iommufd/main.c @@ -180,6 +180,26 @@ struct iommufd_object *iommufd_get_object(struct iommufd_ctx *ictx, u32 id, return obj; } +/* + * Increment the users count of an object outside the context of an ioctl that + * has already locked it. The users refcount cannot be increased on an already + * created object unless the object is installed in the xarray, otherwise things + * are racing with a parallel destruction. + */ +int iommufd_try_inc_users(struct iommufd_ctx *ictx, struct iommufd_object *obj) +{ + struct iommufd_object *cur; + + xa_lock(&ictx->objects); + cur = xa_load(&ictx->objects, obj->id); + if (cur == obj) + refcount_inc(&obj->users); + xa_unlock(&ictx->objects); + if (cur != obj) + return -EBUSY; + return 0; +} + static int iommufd_object_dec_wait(struct iommufd_ctx *ictx, struct iommufd_object *to_destroy) {