3.16.39-rc1 review patch. If anyone has any objections, please let me know.
------------------ From: Rob Clark <[email protected]> commit d78d383ab354b0b9e1d23404ae0d9fbdeb9aa035 upstream. 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 Signed-off-by: Rob Clark <[email protected]> [bwh: Backported to 3.16: adjust context] Signed-off-by: Ben Hutchings <[email protected]> --- 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(+) --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -124,6 +124,12 @@ struct msm_drm_private { */ struct drm_mm mm; } vram; + + /* 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 { --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -188,11 +188,20 @@ int msm_gem_fault(struct vm_area_struct { 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 */ --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -360,6 +360,8 @@ int msm_ioctl_gem_submit(struct drm_devi if (ret) return ret; + priv->struct_mutex_task = current; + submit = submit_create(dev, gpu, args->nr_bos); if (!submit) { ret = -ENOMEM; @@ -442,6 +444,7 @@ out: if (submit) submit_cleanup(submit, !!ret); out_unlock: + priv->struct_mutex_task = NULL; mutex_unlock(&dev->struct_mutex); return ret; }
