From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966078AbcIVRne (ORCPT ); Thu, 22 Sep 2016 13:43:34 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:56936 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030224AbcIVRnY (ORCPT ); Thu, 22 Sep 2016 13:43:24 -0400 From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Rob Clark Subject: [PATCH 4.7 004/184] drm/msm: protect against faults from copy_from_user() in submit ioctl Date: Thu, 22 Sep 2016 19:38:58 +0200 Message-Id: <20160922174048.859624034@linuxfoundation.org> X-Mailer: git-send-email 2.10.0 In-Reply-To: <20160922174048.653794923@linuxfoundation.org> References: <20160922174048.653794923@linuxfoundation.org> User-Agent: quilt/0.64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.7-stable review patch. If anyone has any objections, please let me know. ------------------ From: Rob Clark 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 Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/msm/msm_drv.h | 6 ++++++ drivers/gpu/drm/msm/msm_gem.c | 9 +++++++++ drivers/gpu/drm/msm/msm_gem_submit.c | 2 ++ 3 files changed, 17 insertions(+) --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -148,6 +148,12 @@ struct msm_drm_private { } vram; 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 { --- 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 { 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 @@ -394,6 +394,7 @@ int msm_ioctl_gem_submit(struct drm_devi return -ENOMEM; mutex_lock(&dev->struct_mutex); + priv->struct_mutex_task = current; ret = submit_lookup_objects(submit, args, file); if (ret) @@ -479,6 +480,7 @@ out: submit_cleanup(submit); if (ret) msm_gem_submit_free(submit); + priv->struct_mutex_task = NULL; mutex_unlock(&dev->struct_mutex); return ret; }