aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRob Clark <robdclark@gmail.com>2016-08-22 15:28:38 -0400
committerRob Clark <robdclark@gmail.com>2016-08-28 12:49:39 -0400
commitd78d383ab354b0b9e1d23404ae0d9fbdeb9aa035 (patch)
treeaee4580ca0766d3be40c2b574dd7816aabc3d080
parent89f82cbb0d5c0ab768c8d02914188aa2211cd2e3 (diff)
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: Rob Clark <robdclark@gmail.com>
-rw-r--r--drivers/gpu/drm/msm/msm_drv.h6
-rw-r--r--drivers/gpu/drm/msm/msm_gem.c9
-rw-r--r--drivers/gpu/drm/msm/msm_gem_submit.c3
3 files changed, 18 insertions, 0 deletions
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index b4bc7f1ef717..d0da52f2a806 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 {
157 struct shrinker shrinker; 157 struct shrinker shrinker;
158 158
159 struct msm_vblank_ctrl vblank_ctrl; 159 struct msm_vblank_ctrl vblank_ctrl;
160
161 /* task holding struct_mutex.. currently only used in submit path
162 * to detect and reject faults from copy_from_user() for submit
163 * ioctl.
164 */
165 struct task_struct *struct_mutex_task;
160}; 166};
161 167
162struct msm_format { 168struct msm_format {
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 6cd4af443139..85f3047e05ae 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)
196{ 196{
197 struct drm_gem_object *obj = vma->vm_private_data; 197 struct drm_gem_object *obj = vma->vm_private_data;
198 struct drm_device *dev = obj->dev; 198 struct drm_device *dev = obj->dev;
199 struct msm_drm_private *priv = dev->dev_private;
199 struct page **pages; 200 struct page **pages;
200 unsigned long pfn; 201 unsigned long pfn;
201 pgoff_t pgoff; 202 pgoff_t pgoff;
202 int ret; 203 int ret;
203 204
205 /* This should only happen if userspace tries to pass a mmap'd
206 * but unfaulted gem bo vaddr into submit ioctl, triggering
207 * a page fault while struct_mutex is already held. This is
208 * not a valid use-case so just bail.
209 */
210 if (priv->struct_mutex_task == current)
211 return VM_FAULT_SIGBUS;
212
204 /* Make sure we don't parallel update on a fault, nor move or remove 213 /* Make sure we don't parallel update on a fault, nor move or remove
205 * something from beneath our feet 214 * something from beneath our feet
206 */ 215 */
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 408da409a216..880d6a9af7c8 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -394,6 +394,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
394 if (ret) 394 if (ret)
395 return ret; 395 return ret;
396 396
397 priv->struct_mutex_task = current;
398
397 submit = submit_create(dev, gpu, args->nr_bos, args->nr_cmds); 399 submit = submit_create(dev, gpu, args->nr_bos, args->nr_cmds);
398 if (!submit) { 400 if (!submit) {
399 ret = -ENOMEM; 401 ret = -ENOMEM;
@@ -485,6 +487,7 @@ out:
485 if (ret) 487 if (ret)
486 msm_gem_submit_free(submit); 488 msm_gem_submit_free(submit);
487out_unlock: 489out_unlock:
490 priv->struct_mutex_task = NULL;
488 mutex_unlock(&dev->struct_mutex); 491 mutex_unlock(&dev->struct_mutex);
489 return ret; 492 return ret;
490} 493}