iommufd: WARN if an object is aborted with an elevated refcount

If something holds a refcount then it is at risk of UAFing. For abort
paths we expect the caller to never share the object with a parallel
thread and to clean up any refcounts it obtained on its own.

Add the missing dec inside iommufd_hwpt_paging_alloc() during error unwind
by making iommufd_hw_pagetable_attach/detach() proper pairs.

Link: https://patch.msgid.link/r/2-v1-02cd136829df+31-iommufd_syz_fput_jgg@nvidia.com
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 65fbd098f9e98f2c83dab07f2fe501f2feeb4e27..4c842368289f08dc21b59d9afe7f229c8025f138 100644 (file)
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -711,6 +711,8 @@ iommufd_hw_pagetable_detach(struct iommufd_device *idev, ioasid_t pasid)
                iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, idev->dev);
        mutex_unlock(&igroup->lock);
 
+       iommufd_hw_pagetable_put(idev->ictx, hwpt);
+
        /* Caller must destroy hwpt */
        return hwpt;
 }
@@ -1057,7 +1059,6 @@ void iommufd_device_detach(struct iommufd_device *idev, ioasid_t pasid)
        hwpt = iommufd_hw_pagetable_detach(idev, pasid);
        if (!hwpt)
                return;
-       iommufd_hw_pagetable_put(idev->ictx, hwpt);
        refcount_dec(&idev->obj.users);
 }
 EXPORT_SYMBOL_NS_GPL(iommufd_device_detach, "IOMMUFD");

diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 0da2a81eedfa8bbe7e3236cb1b8526bf6a2fc6dc..627f9b78483a0ea874ca22f65b9466318c869814 100644 (file)
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -454,9 +454,8 @@ static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx,
        if (hwpt->obj.type == IOMMUFD_OBJ_HWPT_PAGING) {
                struct iommufd_hwpt_paging *hwpt_paging = to_hwpt_paging(hwpt);
 
-               lockdep_assert_not_held(&hwpt_paging->ioas->mutex);
-
                if (hwpt_paging->auto_domain) {
+                       lockdep_assert_not_held(&hwpt_paging->ioas->mutex);
                        iommufd_object_put_and_try_destroy(ictx, &hwpt->obj);
                        return;
                }

diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
index 88be2e15724501bb963d607f38c64251b488ff4d..ce775fbbae94e7b9d6c0abdf83a2b482a19ef12a 100644 (file)
--- a/drivers/iommu/iommufd/main.c
+++ b/drivers/iommu/iommufd/main.c
@@ -122,6 +122,10 @@ void iommufd_object_abort(struct iommufd_ctx *ictx, struct iommufd_object *obj)
        old = xas_store(&xas, NULL);
        xa_unlock(&ictx->objects);
        WARN_ON(old != XA_ZERO_ENTRY);
+
+       if (WARN_ON(!refcount_dec_and_test(&obj->users)))
+               return;
+
        kfree(obj);
 }