Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752989AbdLHKQ6 convert rfc822-to-8bit (ORCPT ); Fri, 8 Dec 2017 05:16:58 -0500 Received: from smtp-out6.electric.net ([192.162.217.185]:64339 "EHLO smtp-out6.electric.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751321AbdLHKQy (ORCPT ); Fri, 8 Dec 2017 05:16:54 -0500 From: David Laight To: "'Kees Cook'" , Jason Ekstrand CC: Chris Wilson , "intel-gfx@lists.freedesktop.org" , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" Subject: RE: [PATCH] drm/i915: Use copy_from_user() in fence copying Thread-Topic: [PATCH] drm/i915: Use copy_from_user() in fence copying Thread-Index: AQHTbtDlKQvHN6piYkuli9t0F+yfhaM5PI2A Date: Fri, 8 Dec 2017 10:17:09 +0000 Message-ID: References: <20171206202850.GA38365@beast> In-Reply-To: <20171206202850.GA38365@beast> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.202.205.33] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-Outbound-IP: 156.67.243.126 X-Env-From: David.Laight@ACULAB.COM X-Proto: esmtps X-Revdns: X-HELO: AcuMS.aculab.com X-TLS: TLSv1.2:ECDHE-RSA-AES256-SHA384:256 X-Authenticated_ID: X-PolicySMART: 3396946, 3397078 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1833 Lines: 54 From: Kees Cook > Sent: 06 December 2017 20:29 > > There's no good reason to separate the access_ok() from the copy, > especially since the access_ok() size is hard-coded instead of using > sizeof(). Instead, just use copy_from_user() directly. Looks like an optimisation to save doing the access_ok() check for every 'fence'. OTOH 'user copy hardening' probably makes a much larger performance degradation on this kind of code. (Might be ok here because &fence probably refers to something in the current stack frame.) David > Fixes: cf6e7bac6357 ("drm/i915: Add support for drm syncobjs") > Cc: Jason Ekstrand > Cc: Chris Wilson > Signed-off-by: Kees Cook > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 435ed95df144..1da703213b17 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -2087,8 +2087,6 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args, > return ERR_PTR(-EINVAL); > > user = u64_to_user_ptr(args->cliprects_ptr); > - if (!access_ok(VERIFY_READ, user, nfences * 2 * sizeof(u32))) > - return ERR_PTR(-EFAULT); > > fences = kvmalloc_array(args->num_cliprects, sizeof(*fences), > __GFP_NOWARN | GFP_KERNEL); > @@ -2099,7 +2097,7 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args, > struct drm_i915_gem_exec_fence fence; > struct drm_syncobj *syncobj; > > - if (__copy_from_user(&fence, user++, sizeof(fence))) { > + if (copy_from_user(&fence, user++, sizeof(fence))) { > err = -EFAULT; > goto err; > } > -- > 2.7.4 > > > -- > Kees Cook > Pixel Security