diff options
author | Rob Clark <robdclark@gmail.com> | 2016-08-22 15:28:38 -0400 |
---|---|---|
committer | Rob Clark <robdclark@gmail.com> | 2016-08-28 12:49:39 -0400 |
commit | d78d383ab354b0b9e1d23404ae0d9fbdeb9aa035 (patch) | |
tree | aee4580ca0766d3be40c2b574dd7816aabc3d080 | |
parent | 89f82cbb0d5c0ab768c8d02914188aa2211cd2e3 (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.h | 6 | ||||
-rw-r--r-- | drivers/gpu/drm/msm/msm_gem.c | 9 | ||||
-rw-r--r-- | drivers/gpu/drm/msm/msm_gem_submit.c | 3 |
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 | ||
162 | struct msm_format { | 168 | struct 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); |
487 | out_unlock: | 489 | out_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 | } |