From: Rob Clark <robdclark@gmail.com>
To: dri-devel@lists.freedesktop.org
Cc: Daniel Vetter <daniel@ffwll.ch>,
Vaishali Thakkar <vaishali.thakkar@oracle.com>,
Al Viro <viro@zeniv.linux.org.uk>,
freedreno@lists.freedesktop.org, Rob Clark <robdclark@gmail.com>,
stable@vger.kernel.org
Subject: [PATCH 2/2] drm/msm: protect against faults from copy_from_user() in submit ioctl
Date: Mon, 22 Aug 2016 15:38:05 -0400 [thread overview]
Message-ID: <1471894685-20199-2-git-send-email-robdclark@gmail.com> (raw)
In-Reply-To: <1471894685-20199-1-git-send-email-robdclark@gmail.com>
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: Rob Clark <robdclark@gmail.com>
---
drivers/gpu/drm/msm/msm_drv.h | 6 ++++++
drivers/gpu/drm/msm/msm_gem.c | 9 +++++++++
drivers/gpu/drm/msm/msm_gem_submit.c | 3 +++
3 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index a35c1b6..957801e 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -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 {
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 8dfdeec..f6b8945 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -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
*/
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 03d4ce2..0be57a9 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -426,6 +426,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
if (ret)
return ret;
+ priv->struct_mutex_task = current;
+
if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
if (out_fence_fd < 0) {
@@ -549,6 +551,7 @@ out:
out_unlock:
if (ret && (out_fence_fd >= 0))
put_unused_fd(out_fence_fd);
+ priv->struct_mutex_task = NULL;
mutex_unlock(&dev->struct_mutex);
return ret;
}
--
2.7.4
next prev parent reply other threads:[~2016-08-22 19:38 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-22 19:38 [PATCH 1/2] drm/msm: fix use of copy_from_user() while holding spinlock Rob Clark
2016-08-22 19:38 ` Rob Clark [this message]
2016-08-23 6:03 ` [PATCH 2/2] drm/msm: protect against faults from copy_from_user() in submit ioctl Daniel Vetter
2016-08-28 16:43 ` Rob Clark
2016-08-28 16:53 ` Daniel Vetter
2016-08-28 17:29 ` Rob Clark
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1471894685-20199-2-git-send-email-robdclark@gmail.com \
--to=robdclark@gmail.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=stable@vger.kernel.org \
--cc=vaishali.thakkar@oracle.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox