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: [email protected] > Fixes: 07838f7fd529 ("iommufd: Add iommufd fault object") > Reported-by: [email protected] > Closes: https://lore.kernel.org/r/[email protected] > Signed-off-by: Jason Gunthorpe <[email protected]> The patch looks good to me though: Reviewed-by: Nicolin Chen <[email protected]> > @@ -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
