Commit 20224d71 authored by Rob Clark's avatar Rob Clark
Browse files

drm/msm/submit: Move copy_from_user ahead of locking bos 



We cannot switch to using obj->resv for locking without first moving all
the copy_from_user() ahead of submit_lock_objects().  Otherwise in the
mm fault path we aquire mm->mmap_sem before obj lock, but in the submit
path the order is reversed.

Signed-off-by: default avatarRob Clark <robdclark@chromium.org>
Reviewed-by: default avatarKristian H. Kristensen <hoegsberg@google.com>
Signed-off-by: default avatarRob Clark <robdclark@chromium.org>
parent 599089c6

drivers/gpu/drm/msm/msm_gem.h

+3 −0
Original line number Diff line number Diff line
@@ -240,7 +240,10 @@ struct msm_gem_submit { 
		uint32_t type;

		uint32_t size;  /* in dwords */

		uint64_t iova;

		uint32_t offset;/* in dwords */

		uint32_t idx;   /* cmdstream buffer idx in bos[] */

		uint32_t nr_relocs;

		struct drm_msm_gem_submit_reloc *relocs;

	} *cmd;  /* array of size nr_cmds */

	struct {

		uint32_t flags;

drivers/gpu/drm/msm/msm_gem_submit.c

+79 −48
Original line number Diff line number Diff line
@@ -62,11 +62,16 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, 


void msm_gem_submit_free(struct msm_gem_submit *submit)

{

	unsigned i;



	dma_fence_put(submit->fence);

	list_del(&submit->node);

	put_pid(submit->pid);

	msm_submitqueue_put(submit->queue);



	for (i = 0; i < submit->nr_cmds; i++)

		kfree(submit->cmd[i].relocs);



	kfree(submit);

}
@@ -150,6 +155,66 @@ static int submit_lookup_objects(struct msm_gem_submit *submit, 
	return ret;

}



static int submit_lookup_cmds(struct msm_gem_submit *submit,

		struct drm_msm_gem_submit *args, struct drm_file *file)

{

	unsigned i, sz;

	int ret = 0;



	for (i = 0; i < args->nr_cmds; i++) {

		struct drm_msm_gem_submit_cmd submit_cmd;

		void __user *userptr =

			u64_to_user_ptr(args->cmds + (i * sizeof(submit_cmd)));



		ret = copy_from_user(&submit_cmd, userptr, sizeof(submit_cmd));

		if (ret) {

			ret = -EFAULT;

			goto out;

		}



		/* validate input from userspace: */

		switch (submit_cmd.type) {

		case MSM_SUBMIT_CMD_BUF:

		case MSM_SUBMIT_CMD_IB_TARGET_BUF:

		case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:

			break;

		default:

			DRM_ERROR("invalid type: %08x\n", submit_cmd.type);

			return -EINVAL;

		}



		if (submit_cmd.size % 4) {

			DRM_ERROR("non-aligned cmdstream buffer size: %u\n",

					submit_cmd.size);

			ret = -EINVAL;

			goto out;

		}



		submit->cmd[i].type = submit_cmd.type;

		submit->cmd[i].size = submit_cmd.size / 4;

		submit->cmd[i].offset = submit_cmd.submit_offset / 4;

		submit->cmd[i].idx  = submit_cmd.submit_idx;

		submit->cmd[i].nr_relocs = submit_cmd.nr_relocs;



		sz = array_size(submit_cmd.nr_relocs,

				sizeof(struct drm_msm_gem_submit_reloc));

		/* check for overflow: */

		if (sz == SIZE_MAX) {

			ret = -ENOMEM;

			goto out;

		}

		submit->cmd[i].relocs = kmalloc(sz, GFP_KERNEL);

		ret = copy_from_user(submit->cmd[i].relocs, userptr, sz);

		if (ret) {

			ret = -EFAULT;

			goto out;

		}

	}



out:

	return ret;

}



static void submit_unlock_unpin_bo(struct msm_gem_submit *submit,

		int i, bool backoff)

{
@@ -301,7 +366,7 @@ static int submit_bo(struct msm_gem_submit *submit, uint32_t idx, 


/* process the reloc's and patch up the cmdstream as needed: */

static int submit_reloc(struct msm_gem_submit *submit, struct msm_gem_object *obj,

		uint32_t offset, uint32_t nr_relocs, uint64_t relocs)

		uint32_t offset, uint32_t nr_relocs, struct drm_msm_gem_submit_reloc *relocs)

{

	uint32_t i, last_offset = 0;

	uint32_t *ptr;
@@ -327,18 +392,11 @@ static int submit_reloc(struct msm_gem_submit *submit, struct msm_gem_object *ob 
	}



	for (i = 0; i < nr_relocs; i++) {

		struct drm_msm_gem_submit_reloc submit_reloc;

		void __user *userptr =

			u64_to_user_ptr(relocs + (i * sizeof(submit_reloc)));

		struct drm_msm_gem_submit_reloc submit_reloc = relocs[i];

		uint32_t off;

		uint64_t iova;

		bool valid;



		if (copy_from_user(&submit_reloc, userptr, sizeof(submit_reloc))) {

			ret = -EFAULT;

			goto out;

		}



		if (submit_reloc.submit_offset % 4) {

			DRM_ERROR("non-aligned reloc offset: %u\n",

					submit_reloc.submit_offset);
@@ -694,6 +752,10 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, 
	if (ret)

		goto out;



	ret = submit_lookup_cmds(submit, args, file);

	if (ret)

		goto out;



	/* copy_*_user while holding a ww ticket upsets lockdep */

	ww_acquire_init(&submit->ticket, &reservation_ww_class);

	has_ww_ticket = true;
@@ -710,60 +772,29 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, 
		goto out;



	for (i = 0; i < args->nr_cmds; i++) {

		struct drm_msm_gem_submit_cmd submit_cmd;

		void __user *userptr =

			u64_to_user_ptr(args->cmds + (i * sizeof(submit_cmd)));

		struct msm_gem_object *msm_obj;

		uint64_t iova;



		ret = copy_from_user(&submit_cmd, userptr, sizeof(submit_cmd));

		if (ret) {

			ret = -EFAULT;

			goto out;

		}



		/* validate input from userspace: */

		switch (submit_cmd.type) {

		case MSM_SUBMIT_CMD_BUF:

		case MSM_SUBMIT_CMD_IB_TARGET_BUF:

		case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:

			break;

		default:

			DRM_ERROR("invalid type: %08x\n", submit_cmd.type);

			ret = -EINVAL;

			goto out;

		}



		ret = submit_bo(submit, submit_cmd.submit_idx,

		ret = submit_bo(submit, submit->cmd[i].idx,

				&msm_obj, &iova, NULL);

		if (ret)

			goto out;



		if (submit_cmd.size % 4) {

			DRM_ERROR("non-aligned cmdstream buffer size: %u\n",

					submit_cmd.size);

		if (!submit->cmd[i].size ||

			((submit->cmd[i].size + submit->cmd[i].offset) >

				msm_obj->base.size / 4)) {

			DRM_ERROR("invalid cmdstream size: %u\n", submit->cmd[i].size * 4);

			ret = -EINVAL;

			goto out;

		}



		if (!submit_cmd.size ||

			((submit_cmd.size + submit_cmd.submit_offset) >

				msm_obj->base.size)) {

			DRM_ERROR("invalid cmdstream size: %u\n", submit_cmd.size);

			ret = -EINVAL;

			goto out;

		}



		submit->cmd[i].type = submit_cmd.type;

		submit->cmd[i].size = submit_cmd.size / 4;

		submit->cmd[i].iova = iova + submit_cmd.submit_offset;

		submit->cmd[i].idx  = submit_cmd.submit_idx;

		submit->cmd[i].iova = iova + (submit->cmd[i].offset * 4);



		if (submit->valid)

			continue;



		ret = submit_reloc(submit, msm_obj, submit_cmd.submit_offset,

				submit_cmd.nr_relocs, submit_cmd.relocs);

		ret = submit_reloc(submit, msm_obj, submit->cmd[i].offset * 4,

				submit->cmd[i].nr_relocs, submit->cmd[i].relocs);

		if (ret)

			goto out;

	}