Commit d4c7fccf authored by Linus Torvalds's avatar Linus Torvalds
Browse files
Pull iommufd fixes from Jason Gunthorpe:
 "Fix two user triggerable use-after-free issues:

   - Possible race UAF setting up mmaps

   - Syzkaller found UAF when erroring an file descriptor creation ioctl
     due to the fput() work queue"

* tag 'for-linus-iommufd' of git://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd:
  iommufd/selftest: Update the fail_nth limit
  iommufd: WARN if an object is aborted with an elevated refcount
  iommufd: Fix race during abort for file descriptors
  iommufd: Fix refcounting race during mmap
parents b183f251 43f6bee0
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -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");
+2 −7
Original line number Diff line number Diff line
@@ -393,12 +393,12 @@ static int iommufd_eventq_init(struct iommufd_eventq *eventq, char *name,
			       const struct file_operations *fops)
{
	struct file *filep;
	int fdno;

	spin_lock_init(&eventq->lock);
	INIT_LIST_HEAD(&eventq->deliver);
	init_waitqueue_head(&eventq->wait_queue);

	/* The filep is fput() by the core code during failure */
	filep = anon_inode_getfile(name, fops, eventq, O_RDWR);
	if (IS_ERR(filep))
		return PTR_ERR(filep);
@@ -408,10 +408,7 @@ static int iommufd_eventq_init(struct iommufd_eventq *eventq, char *name,
	eventq->filep = filep;
	refcount_inc(&eventq->obj.users);

	fdno = get_unused_fd_flags(O_CLOEXEC);
	if (fdno < 0)
		fput(filep);
	return fdno;
	return get_unused_fd_flags(O_CLOEXEC);
}

static const struct file_operations iommufd_fault_fops =
@@ -452,7 +449,6 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd)
	return 0;
out_put_fdno:
	put_unused_fd(fdno);
	fput(fault->common.filep);
	return rc;
}

@@ -536,7 +532,6 @@ int iommufd_veventq_alloc(struct iommufd_ucmd *ucmd)

out_put_fdno:
	put_unused_fd(fdno);
	fput(veventq->common.filep);
out_abort:
	iommufd_object_abort_and_destroy(ucmd->ictx, &veventq->common.obj);
out_unlock_veventqs:
+1 −2
Original line number Diff line number Diff line
@@ -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;
		}
+50 −9
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@
#include "iommufd_test.h"

struct iommufd_object_ops {
	size_t file_offset;
	void (*pre_destroy)(struct iommufd_object *obj);
	void (*destroy)(struct iommufd_object *obj);
	void (*abort)(struct iommufd_object *obj);
@@ -121,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);
}

@@ -131,10 +136,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);
}

@@ -550,16 +575,23 @@ static int iommufd_fops_mmap(struct file *filp, struct vm_area_struct *vma)
	if (vma->vm_flags & VM_EXEC)
		return -EPERM;

	mtree_lock(&ictx->mt_mmap);
	/* vma->vm_pgoff carries a page-shifted start position to an immap */
	immap = mtree_load(&ictx->mt_mmap, vma->vm_pgoff << PAGE_SHIFT);
	if (!immap)
	if (!immap || !refcount_inc_not_zero(&immap->owner->users)) {
		mtree_unlock(&ictx->mt_mmap);
		return -ENXIO;
	}
	mtree_unlock(&ictx->mt_mmap);

	/*
	 * mtree_load() returns the immap for any contained mmio_addr, so only
	 * allow the exact immap thing to be mapped
	 */
	if (vma->vm_pgoff != immap->vm_pgoff || length != immap->length)
		return -ENXIO;
	if (vma->vm_pgoff != immap->vm_pgoff || length != immap->length) {
		rc = -ENXIO;
		goto err_refcount;
	}

	vma->vm_pgoff = 0;
	vma->vm_private_data = immap;
@@ -570,10 +602,11 @@ static int iommufd_fops_mmap(struct file *filp, struct vm_area_struct *vma)
				immap->mmio_addr >> PAGE_SHIFT, length,
				vma->vm_page_prot);
	if (rc)
		return rc;
		goto err_refcount;
	return 0;

	/* vm_ops.open won't be called for mmap itself. */
	refcount_inc(&immap->owner->users);
err_refcount:
	refcount_dec(&immap->owner->users);
	return rc;
}

@@ -651,6 +684,12 @@ void iommufd_ctx_put(struct iommufd_ctx *ictx)
}
EXPORT_SYMBOL_NS_GPL(iommufd_ctx_put, "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,
@@ -661,6 +700,7 @@ static const struct iommufd_object_ops iommufd_object_ops[] = {
	},
	[IOMMUFD_OBJ_FAULT] = {
		.destroy = iommufd_fault_destroy,
		IOMMUFD_FILE_OFFSET(struct iommufd_fault, common.filep, common.obj),
	},
	[IOMMUFD_OBJ_HW_QUEUE] = {
		.destroy = iommufd_hw_queue_destroy,
@@ -683,6 +723,7 @@ static const struct iommufd_object_ops iommufd_object_ops[] = {
	[IOMMUFD_OBJ_VEVENTQ] = {
		.destroy = iommufd_veventq_destroy,
		.abort = iommufd_veventq_abort,
		IOMMUFD_FILE_OFFSET(struct iommufd_veventq, common.filep, common.obj),
	},
	[IOMMUFD_OBJ_VIOMMU] = {
		.destroy = iommufd_viommu_destroy,
+1 −1
Original line number Diff line number Diff line
@@ -113,7 +113,7 @@ static bool fail_nth_next(struct __test_metadata *_metadata,
	 * necessarily mean a test failure, just that the limit has to be made
	 * bigger.
	 */
	ASSERT_GT(400, nth_state->iteration);
	ASSERT_GT(1000, nth_state->iteration);
	if (nth_state->iteration != 0) {
		ssize_t res;
		ssize_t res2;