Commit b673499a authored by Kristian H. Kristensen's avatar Kristian H. Kristensen Committed by Rob Clark
Browse files

drm/msm: Split submit_lookup_objects() into two loops



First loop does copy_from_user() without the table lock held and
just stores the handle. Second loop looks up buffer objects with the
table_lock held without potentially blocking or faulting. This lets us
clean up a bunch of custom, non-faulting copy_from_user() code.

Signed-off-by: default avatarKristian H. Kristensen <hoegsberg@chromium.org>
Signed-off-by: default avatarRob Clark <robdclark@chromium.org>
parent 8ea274ac
Loading
Loading
Loading
Loading
+4 −1
Original line number Diff line number Diff line
@@ -166,7 +166,10 @@ struct msm_gem_submit {
	} *cmd;  /* array of size nr_cmds */
	struct {
		uint32_t flags;
		union {
			struct msm_gem_object *obj;
			uint32_t handle;
		};
		uint64_t iova;
	} bos[0];
};
+17 −27
Original line number Diff line number Diff line
@@ -74,27 +74,14 @@ void msm_gem_submit_free(struct msm_gem_submit *submit)
	kfree(submit);
}

static inline unsigned long __must_check
copy_from_user_inatomic(void *to, const void __user *from, unsigned long n)
{
	if (access_ok(from, n))
		return __copy_from_user_inatomic(to, from, n);
	return -EFAULT;
}

static int submit_lookup_objects(struct msm_gem_submit *submit,
		struct drm_msm_gem_submit *args, struct drm_file *file)
{
	unsigned i;
	int ret = 0;

	spin_lock(&file->table_lock);
	pagefault_disable();

	for (i = 0; i < args->nr_bos; i++) {
		struct drm_msm_gem_submit_bo submit_bo;
		struct drm_gem_object *obj;
		struct msm_gem_object *msm_obj;
		void __user *userptr =
			u64_to_user_ptr(args->bos + (i * sizeof(submit_bo)));

@@ -103,16 +90,11 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
		 */
		submit->bos[i].flags = 0;

		if (copy_from_user_inatomic(&submit_bo, userptr, sizeof(submit_bo))) {
			pagefault_enable();
			spin_unlock(&file->table_lock);
		if (copy_from_user(&submit_bo, userptr, sizeof(submit_bo))) {
			ret = -EFAULT;
			i = 0;
			goto out;
		}
			spin_lock(&file->table_lock);
			pagefault_disable();
		}

/* at least one of READ and/or WRITE flags should be set: */
#define MANDATORY_FLAGS (MSM_SUBMIT_BO_READ | MSM_SUBMIT_BO_WRITE)
@@ -121,19 +103,28 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
			!(submit_bo.flags & MANDATORY_FLAGS)) {
			DRM_ERROR("invalid flags: %x\n", submit_bo.flags);
			ret = -EINVAL;
			goto out_unlock;
			i = 0;
			goto out;
		}

		submit->bos[i].handle = submit_bo.handle;
		submit->bos[i].flags = submit_bo.flags;
		/* in validate_objects() we figure out if this is true: */
		submit->bos[i].iova  = submit_bo.presumed;
	}

	spin_lock(&file->table_lock);

	for (i = 0; i < args->nr_bos; i++) {
		struct drm_gem_object *obj;
		struct msm_gem_object *msm_obj;

		/* normally use drm_gem_object_lookup(), but for bulk lookup
		 * all under single table_lock just hit object_idr directly:
		 */
		obj = idr_find(&file->object_idr, submit_bo.handle);
		obj = idr_find(&file->object_idr, submit->bos[i].handle);
		if (!obj) {
			DRM_ERROR("invalid handle %u at index %u\n", submit_bo.handle, i);
			DRM_ERROR("invalid handle %u at index %u\n", submit->bos[i].handle, i);
			ret = -EINVAL;
			goto out_unlock;
		}
@@ -142,7 +133,7 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,

		if (!list_empty(&msm_obj->submit_entry)) {
			DRM_ERROR("handle %u at index %u already on submit list\n",
					submit_bo.handle, i);
					submit->bos[i].handle, i);
			ret = -EINVAL;
			goto out_unlock;
		}
@@ -155,7 +146,6 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
	}

out_unlock:
	pagefault_enable();
	spin_unlock(&file->table_lock);

out: