Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d78d383a authored by Rob Clark's avatar Rob Clark
Browse files

drm/msm: protect against faults from copy_from_user() in submit ioctl

An evil userspace could try to cause deadlock by passing an unfaulted-in
GEM bo as submit->bos (or submit->cmds) table.  Which will trigger
msm_gem_fault() while we already hold struct_mutex.  See:

https://github.com/freedreno/msmtest/blob/master/evilsubmittest.c



Cc: stable@vger.kernel.org
Signed-off-by: default avatarRob Clark <robdclark@gmail.com>
parent 89f82cbb
Loading
Loading
Loading
Loading
+6 −0
Original line number Diff line number Diff line
@@ -157,6 +157,12 @@ struct msm_drm_private {
	struct shrinker shrinker;

	struct msm_vblank_ctrl vblank_ctrl;

	/* task holding struct_mutex.. currently only used in submit path
	 * to detect and reject faults from copy_from_user() for submit
	 * ioctl.
	 */
	struct task_struct *struct_mutex_task;
};

struct msm_format {
+9 −0
Original line number Diff line number Diff line
@@ -196,11 +196,20 @@ int msm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
	struct drm_gem_object *obj = vma->vm_private_data;
	struct drm_device *dev = obj->dev;
	struct msm_drm_private *priv = dev->dev_private;
	struct page **pages;
	unsigned long pfn;
	pgoff_t pgoff;
	int ret;

	/* This should only happen if userspace tries to pass a mmap'd
	 * but unfaulted gem bo vaddr into submit ioctl, triggering
	 * a page fault while struct_mutex is already held.  This is
	 * not a valid use-case so just bail.
	 */
	if (priv->struct_mutex_task == current)
		return VM_FAULT_SIGBUS;

	/* Make sure we don't parallel update on a fault, nor move or remove
	 * something from beneath our feet
	 */
+3 −0
Original line number Diff line number Diff line
@@ -394,6 +394,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
	if (ret)
		return ret;

	priv->struct_mutex_task = current;

	submit = submit_create(dev, gpu, args->nr_bos, args->nr_cmds);
	if (!submit) {
		ret = -ENOMEM;
@@ -485,6 +487,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
	if (ret)
		msm_gem_submit_free(submit);
out_unlock:
	priv->struct_mutex_task = NULL;
	mutex_unlock(&dev->struct_mutex);
	return ret;
}