From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755484AbcKNC21 (ORCPT ); Sun, 13 Nov 2016 21:28:27 -0500 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:47178 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754433AbcKNC2V (ORCPT ); Sun, 13 Nov 2016 21:28:21 -0500 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 From: Ben Hutchings To: linux-kernel@vger.kernel.org, stable@vger.kernel.org CC: akpm@linux-foundation.org, "Rob Clark" Date: Mon, 14 Nov 2016 00:14:20 +0000 Message-ID: X-Mailer: LinuxStableQueue (scripts by bwh) Subject: [PATCH 3.16 229/346] drm/msm: protect against faults from copy_from_user() in submit ioctl In-Reply-To: X-SA-Exim-Connect-IP: 2a02:8011:400e:2:6f00:88c8:c921:d332 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 3.16.39-rc1 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 [bwh: Backported to 3.16: adjust context] Signed-off-by: Ben Hutchings --- 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; }