mainline inclusion from mainline-v6.17-rc1 commit 4e034bf045b12852a24d5d33f2451850818ba0c1 category: bugfix bugzilla: https://atomgit.com/src-openeuler/kernel/issues/7960 CVE: CVE-2025-39966 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- fput() doesn't actually call file_operations release() synchronously, it puts the file on a work queue and it will be released eventually. This is normally fine, except for iommufd the file and the iommufd_object are tied to gether. The file has the object as it's private_data and holds a users refcount, while the object is expected to remain alive as long as the file is. When the allocation of a new object aborts before installing the file it will fput() the file and then go on to immediately kfree() the obj. This causes a UAF once the workqueue completes the fput() and tries to decrement the users refcount. 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. Instead the objects tell the core code where the file pointer is and the core will take care of the life cycle. Adapted from upstream commit 4e034bf045b1 ("iommufd: Fix race during abort for file descriptors"). Differences vs upstream: - No eventq.c/veventq/hw_queue in this tree; the eventq hunks fall to fault.c iommufd_fault_alloc() error paths directly. - struct iommufd_fault holds obj/filep directly (no common substruct). - No pre_destroy member in struct iommufd_object_ops. Link: https://patch.msgid.link/r/1-v1-02cd136829df+31-iommufd_syz_fput_jgg@nvidia.... Cc: stable@vger.kernel.org Fixes: 07838f7fd529 ("iommufd: Add iommufd fault object") Reviewed-by: Nicolin Chen <nicolinc@nvidia.com> Reviewed-by: Nirmoy Das <nirmoyd@nvidia.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Tested-by: Nicolin Chen <nicolinc@nvidia.com> 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> Conflicts: drivers/iommu/iommufd/eventq.c drivers/iommu/iommufd/main.c [context conflict] Signed-off-by: Zhang Yuwei <zhangyuwei20@huawei.com> --- drivers/iommu/iommufd/fault.c | 5 ++--- drivers/iommu/iommufd/main.c | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c index b6a669d06110..8c41c49be2c6 100644 --- a/drivers/iommu/iommufd/fault.c +++ b/drivers/iommu/iommufd/fault.c @@ -401,6 +401,7 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd) mutex_init(&fault->mutex); init_waitqueue_head(&fault->wait_queue); + /* The filep is fput() by the core code during failure */ filep = anon_inode_getfile("[iommufd-pgfault]", &iommufd_fault_fops, fault, O_RDWR); if (IS_ERR(filep)) { @@ -415,7 +416,7 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd) fdno = get_unused_fd_flags(O_CLOEXEC); if (fdno < 0) { rc = fdno; - goto out_fput; + goto out_abort; } cmd->out_fault_id = fault->obj.id; @@ -431,8 +432,6 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd) return 0; out_put_fdno: put_unused_fd(fdno); -out_fput: - fput(filep); out_abort: iommufd_object_abort_and_destroy(ucmd->ictx, &fault->obj); diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c index dc5353e30ac2..35f570875f65 100644 --- a/drivers/iommu/iommufd/main.c +++ b/drivers/iommu/iommufd/main.c @@ -23,6 +23,7 @@ #include "iommufd_test.h" struct iommufd_object_ops { + size_t file_offset; void (*destroy)(struct iommufd_object *obj); void (*abort)(struct iommufd_object *obj); }; @@ -71,10 +72,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; + + /* + * A file should hold a users refcount while the file is open + * and put it back in its release. The file should hold a + * pointer to obj in their private data. Normal fput() is + * deferred to a 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. + */ + if (*filep) + __fput_sync(*filep); + } + + if (ops->abort) + ops->abort(obj); else - iommufd_object_ops[obj->type].destroy(obj); + ops->destroy(obj); iommufd_object_abort(ictx, obj); } @@ -497,6 +518,12 @@ void iommufd_ctx_set_kvm(struct iommufd_ctx *ictx, struct kvm *kvm) } EXPORT_SYMBOL_NS_GPL(iommufd_ctx_set_kvm, IOMMUFD); +#define IOMMUFD_FILE_OFFSET(_struct, _filep, _obj) \ + .file_offset = (offsetof(_struct, _filep) + \ + BUILD_BUG_ON_ZERO(!__same_type( \ + struct file *, ((_struct *)NULL)->_filep)) + \ + BUILD_BUG_ON_ZERO(offsetof(_struct, _obj))) + static const struct iommufd_object_ops iommufd_object_ops[] = { [IOMMUFD_OBJ_ACCESS] = { .destroy = iommufd_access_destroy_object, @@ -517,6 +544,7 @@ static const struct iommufd_object_ops iommufd_object_ops[] = { }, [IOMMUFD_OBJ_FAULT] = { .destroy = iommufd_fault_destroy, + IOMMUFD_FILE_OFFSET(struct iommufd_fault, filep, obj), }, [IOMMUFD_OBJ_VIOMMU] = { .destroy = iommufd_viommu_destroy, -- 2.22.0