Re: [PATCH 1/3] iommufd: Fix race during abort for file descriptors

Nicolin Chen
2025-09-18T05:08:00Z

On Wed, Sep 17, 2025 at 05:01:47PM -0300, Jason Gunthorpe wrote:
> Fix this by putting the core code in charge of the file lifetime, and call
> __fput_sync() during abort to ensure that release() is called before
> kfree. __fput_sync() is a bit too tricky to open code in all the object
> implementations

Mind elaborating this "too tricky"? I thought that we're supposed
to use __fput_sync(), instead of fput(), in the alloc function in
the first place?

> Cc: stable@vger.kernel.org
> Fixes: 07838f7fd529 ("iommufd: Add iommufd fault object")
> Reported-by: syzbot+80620e2d0d0a33b09f93@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/r/68c8583d.050a0220.2ff435.03a2.GAE@google.com
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

The patch looks good to me though:

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

> @@ -131,10 +132,30 @@ void iommufd_object_abort(struct iommufd_ctx *ictx, struct iommufd_object *obj)
> void iommufd_object_abort_and_destroy(struct iommufd_ctx *ictx,
> struct iommufd_object *obj)
> {
> - if (iommufd_object_ops[obj->type].abort)
> - iommufd_object_ops[obj->type].abort(obj);
> + const struct iommufd_object_ops *ops = &iommufd_object_ops[obj->type];
> +
> + if (ops->file_offset) {
> + struct file **filep = ((void *)obj) + ops->file_offset;
> +
> + /*
> + * files should hold a users refcount while the file is open and
> + * put it back in their release. They should hold a pointer to
> + * obj in their private data. Normal fput() is deferred to a

Nit: there is only one file_offset per obj, so it should be "file"
and "it/its"?

> + * workqueue and can get out of order with the following
> + * kfree(obj). Using the sync version ensures the release
> + * happens immediately. During abort we require the file
> + * refcount is one at this point - meaning the object alloc
> + * function cannot do anything to allow another thread to take a
> + * refcount prior to a guaranteed success.
> + */

Thanks
Nicolin