From patchwork Fri Jul 3 11:17:29 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jarkko Sakkinen X-Patchwork-Id: 6714221 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 3D8349F2F0 for ; Fri, 3 Jul 2015 11:17:39 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F257120654 for ; Fri, 3 Jul 2015 11:17:37 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 98AF3203F7 for ; Fri, 3 Jul 2015 11:17:36 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AF9B24A048; Fri, 3 Jul 2015 04:17:35 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id 998FC4A048 for ; Fri, 3 Jul 2015 04:17:34 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 03 Jul 2015 04:17:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,399,1432623600"; d="scan'208";a="739987615" Received: from jsakkine-mobl1.tm.intel.com (HELO localhost) ([10.237.48.25]) by fmsmga001.fm.intel.com with ESMTP; 03 Jul 2015 04:17:32 -0700 From: Jarkko Sakkinen To: airlied@linux.ie, dri-devel@lists.freedesktop.org Subject: [PATCH v2] drm: remove redundant code form drm_ioc32.c Date: Fri, 3 Jul 2015 14:17:29 +0300 Message-Id: <1435922249-6430-1-git-send-email-jarkko.sakkinen@linux.intel.com> X-Mailer: git-send-email 2.1.4 Cc: open list , Jarkko Sakkinen X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The compat ioctl handler ends up calling access_ok() twice: first indirectly inside compat_alloc_user_space() and then after returning from that function. This patch fixes issue. v2: there were three invalid removals of access_ok() that I've fixed. Also went through all the changes couple of times and verified that access_ok() is only removed when the buffer is allocated with compat_alloc_user_space(). My deepest apologies for this kind of sloppiness! Signed-off-by: Jarkko Sakkinen Reviewed-by: Jani Nikula --- drivers/gpu/drm/drm_ioc32.c | 55 +++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c index aa8bbb4..8dcfa76 100644 --- a/drivers/gpu/drm/drm_ioc32.c +++ b/drivers/gpu/drm/drm_ioc32.c @@ -93,7 +93,7 @@ static int compat_drm_version(struct file *file, unsigned int cmd, return -EFAULT; version = compat_alloc_user_space(sizeof(*version)); - if (!access_ok(VERIFY_WRITE, version, sizeof(*version))) + if (!version) return -EFAULT; if (__put_user(v32.name_len, &version->name_len) || __put_user((void __user *)(unsigned long)v32.name, @@ -140,7 +140,7 @@ static int compat_drm_getunique(struct file *file, unsigned int cmd, return -EFAULT; u = compat_alloc_user_space(sizeof(*u)); - if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) + if (!u) return -EFAULT; if (__put_user(uq32.unique_len, &u->unique_len) || __put_user((void __user *)(unsigned long)uq32.unique, @@ -168,7 +168,7 @@ static int compat_drm_setunique(struct file *file, unsigned int cmd, return -EFAULT; u = compat_alloc_user_space(sizeof(*u)); - if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) + if (!u) return -EFAULT; if (__put_user(uq32.unique_len, &u->unique_len) || __put_user((void __user *)(unsigned long)uq32.unique, @@ -200,7 +200,7 @@ static int compat_drm_getmap(struct file *file, unsigned int cmd, return -EFAULT; map = compat_alloc_user_space(sizeof(*map)); - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) + if (!map) return -EFAULT; if (__put_user(idx, &map->offset)) return -EFAULT; @@ -237,7 +237,7 @@ static int compat_drm_addmap(struct file *file, unsigned int cmd, return -EFAULT; map = compat_alloc_user_space(sizeof(*map)); - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) + if (!map) return -EFAULT; if (__put_user(m32.offset, &map->offset) || __put_user(m32.size, &map->size) @@ -277,7 +277,7 @@ static int compat_drm_rmmap(struct file *file, unsigned int cmd, return -EFAULT; map = compat_alloc_user_space(sizeof(*map)); - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) + if (!map) return -EFAULT; if (__put_user((void *)(unsigned long)handle, &map->handle)) return -EFAULT; @@ -306,7 +306,7 @@ static int compat_drm_getclient(struct file *file, unsigned int cmd, return -EFAULT; client = compat_alloc_user_space(sizeof(*client)); - if (!access_ok(VERIFY_WRITE, client, sizeof(*client))) + if (!client) return -EFAULT; if (__put_user(idx, &client->idx)) return -EFAULT; @@ -345,7 +345,7 @@ static int compat_drm_getstats(struct file *file, unsigned int cmd, int i, err; stats = compat_alloc_user_space(sizeof(*stats)); - if (!access_ok(VERIFY_WRITE, stats, sizeof(*stats))) + if (!stats) return -EFAULT; err = drm_ioctl(file, DRM_IOCTL_GET_STATS, (unsigned long)stats); @@ -382,8 +382,7 @@ static int compat_drm_addbufs(struct file *file, unsigned int cmd, unsigned long agp_start; buf = compat_alloc_user_space(sizeof(*buf)); - if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf)) - || !access_ok(VERIFY_WRITE, argp, sizeof(*argp))) + if (!buf || !access_ok(VERIFY_WRITE, argp, sizeof(*argp))) return -EFAULT; if (__copy_in_user(buf, argp, offsetof(drm_buf_desc32_t, agp_start)) @@ -414,7 +413,7 @@ static int compat_drm_markbufs(struct file *file, unsigned int cmd, return -EFAULT; buf = compat_alloc_user_space(sizeof(*buf)); - if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf))) + if (!buf) return -EFAULT; if (__put_user(b32.size, &buf->size) @@ -455,7 +454,7 @@ static int compat_drm_infobufs(struct file *file, unsigned int cmd, nbytes = sizeof(*request) + count * sizeof(struct drm_buf_desc); request = compat_alloc_user_space(nbytes); - if (!access_ok(VERIFY_WRITE, request, nbytes)) + if (!request) return -EFAULT; list = (struct drm_buf_desc *) (request + 1); @@ -516,7 +515,7 @@ static int compat_drm_mapbufs(struct file *file, unsigned int cmd, return -EINVAL; nbytes = sizeof(*request) + count * sizeof(struct drm_buf_pub); request = compat_alloc_user_space(nbytes); - if (!access_ok(VERIFY_WRITE, request, nbytes)) + if (!request) return -EFAULT; list = (struct drm_buf_pub *) (request + 1); @@ -563,7 +562,7 @@ static int compat_drm_freebufs(struct file *file, unsigned int cmd, return -EFAULT; request = compat_alloc_user_space(sizeof(*request)); - if (!access_ok(VERIFY_WRITE, request, sizeof(*request))) + if (!request) return -EFAULT; if (__put_user(req32.count, &request->count) || __put_user((int __user *)(unsigned long)req32.list, @@ -589,7 +588,7 @@ static int compat_drm_setsareactx(struct file *file, unsigned int cmd, return -EFAULT; request = compat_alloc_user_space(sizeof(*request)); - if (!access_ok(VERIFY_WRITE, request, sizeof(*request))) + if (!request) return -EFAULT; if (__put_user(req32.ctx_id, &request->ctx_id) || __put_user((void *)(unsigned long)req32.handle, @@ -613,7 +612,7 @@ static int compat_drm_getsareactx(struct file *file, unsigned int cmd, return -EFAULT; request = compat_alloc_user_space(sizeof(*request)); - if (!access_ok(VERIFY_WRITE, request, sizeof(*request))) + if (!request) return -EFAULT; if (__put_user(ctx_id, &request->ctx_id)) return -EFAULT; @@ -646,7 +645,7 @@ static int compat_drm_resctx(struct file *file, unsigned int cmd, return -EFAULT; res = compat_alloc_user_space(sizeof(*res)); - if (!access_ok(VERIFY_WRITE, res, sizeof(*res))) + if (!res) return -EFAULT; if (__put_user(res32.count, &res->count) || __put_user((struct drm_ctx __user *) (unsigned long)res32.contexts, @@ -689,7 +688,7 @@ static int compat_drm_dma(struct file *file, unsigned int cmd, return -EFAULT; d = compat_alloc_user_space(sizeof(*d)); - if (!access_ok(VERIFY_WRITE, d, sizeof(*d))) + if (!d) return -EFAULT; if (__put_user(d32.context, &d->context) @@ -764,7 +763,7 @@ static int compat_drm_agp_info(struct file *file, unsigned int cmd, int err; info = compat_alloc_user_space(sizeof(*info)); - if (!access_ok(VERIFY_WRITE, info, sizeof(*info))) + if (!info) return -EFAULT; err = drm_ioctl(file, DRM_IOCTL_AGP_INFO, (unsigned long)info); @@ -807,7 +806,7 @@ static int compat_drm_agp_alloc(struct file *file, unsigned int cmd, return -EFAULT; request = compat_alloc_user_space(sizeof(*request)); - if (!access_ok(VERIFY_WRITE, request, sizeof(*request)) + if (!request || __put_user(req32.size, &request->size) || __put_user(req32.type, &request->type)) return -EFAULT; @@ -834,7 +833,7 @@ static int compat_drm_agp_free(struct file *file, unsigned int cmd, u32 handle; request = compat_alloc_user_space(sizeof(*request)); - if (!access_ok(VERIFY_WRITE, request, sizeof(*request)) + if (!request || get_user(handle, &argp->handle) || __put_user(handle, &request->handle)) return -EFAULT; @@ -858,7 +857,7 @@ static int compat_drm_agp_bind(struct file *file, unsigned int cmd, return -EFAULT; request = compat_alloc_user_space(sizeof(*request)); - if (!access_ok(VERIFY_WRITE, request, sizeof(*request)) + if (!request || __put_user(req32.handle, &request->handle) || __put_user(req32.offset, &request->offset)) return -EFAULT; @@ -874,7 +873,7 @@ static int compat_drm_agp_unbind(struct file *file, unsigned int cmd, u32 handle; request = compat_alloc_user_space(sizeof(*request)); - if (!access_ok(VERIFY_WRITE, request, sizeof(*request)) + if (!request || get_user(handle, &argp->handle) || __put_user(handle, &request->handle)) return -EFAULT; @@ -897,8 +896,7 @@ static int compat_drm_sg_alloc(struct file *file, unsigned int cmd, unsigned long x; request = compat_alloc_user_space(sizeof(*request)); - if (!access_ok(VERIFY_WRITE, request, sizeof(*request)) - || !access_ok(VERIFY_WRITE, argp, sizeof(*argp)) + if (!request || !access_ok(VERIFY_WRITE, argp, sizeof(*argp)) || __get_user(x, &argp->size) || __put_user(x, &request->size)) return -EFAULT; @@ -923,8 +921,7 @@ static int compat_drm_sg_free(struct file *file, unsigned int cmd, unsigned long x; request = compat_alloc_user_space(sizeof(*request)); - if (!access_ok(VERIFY_WRITE, request, sizeof(*request)) - || !access_ok(VERIFY_WRITE, argp, sizeof(*argp)) + if (!request || !access_ok(VERIFY_WRITE, argp, sizeof(*argp)) || __get_user(x, &argp->handle) || __put_user(x << PAGE_SHIFT, &request->handle)) return -EFAULT; @@ -952,7 +949,7 @@ static int compat_drm_update_draw(struct file *file, unsigned int cmd, return -EFAULT; request = compat_alloc_user_space(sizeof(*request)); - if (!access_ok(VERIFY_WRITE, request, sizeof(*request)) || + if (!request || __put_user(update32.handle, &request->handle) || __put_user(update32.type, &request->type) || __put_user(update32.num, &request->num) || @@ -994,7 +991,7 @@ static int compat_drm_wait_vblank(struct file *file, unsigned int cmd, return -EFAULT; request = compat_alloc_user_space(sizeof(*request)); - if (!access_ok(VERIFY_WRITE, request, sizeof(*request)) + if (!request || __put_user(req32.request.type, &request->request.type) || __put_user(req32.request.sequence, &request->request.sequence) || __put_user(req32.request.signal, &request->request.signal))