From patchwork Tue May 22 08:09:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jens Wiklander X-Patchwork-Id: 136520 Delivered-To: patches@linaro.org Received: by 2002:a2e:9706:0:0:0:0:0 with SMTP id r6-v6csp1308585lji; Tue, 22 May 2018 01:09:48 -0700 (PDT) X-Received: by 2002:a19:4c08:: with SMTP id z8-v6mr668907lfa.56.1526976588895; Tue, 22 May 2018 01:09:48 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1526976588; cv=none; d=google.com; s=arc-20160816; b=nrV3QrRITTGfguEww5MTLjUZjhO7Y1EZM5RNzf90cVgb9R4pUb+yHEnI1QGqKp5/Zg PVtwI19Ndgqb8obN+c7y02XCaMSeclIoxlDTA7GqMjgG5Gy3PwpMwoRyxFLeWmZbVaH7 aKFgcfmCKf6sICE5v9zSlrwoXYxU6t/6uS0BAix/1t7DGL/fFImLlMtJT1TkLVC5MU/b koS55+vIz1Bvo+ChMA3wp+W3uTBXHSHihVS7+aoGkeyaaC573sp3CG3uNTOAQXbigQuJ Vv/8GARz/gUCUtIXwQZeD+Z5mFqRIZfXLdngeyhFj/9BG6Ce5Tl3tn5PmX7RJZGXmIMo 4S8A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=message-id:date:subject:cc:to:from:dkim-signature :arc-authentication-results; bh=tyDPsf+Vtg1tzo9IFda8SAQnXbVn/CAh7JVSthamDbk=; b=g69M3PLug5GTYfy2rtWgrXXcBBM6W+rAITv9Tq/ct0m+y2r7/mw5kgyf5D3pOXy5ru jF/mpwkg9Q3g3dOT5fO9Yx6tBmLtCe6TVXjj+YAe5I2fAjdLG0/Lf+Gjj30JcJQXbdNZ U6kFgZ2kiOx9eIHhF8QfSul3/MdxhH9Mt0GwMPev8ICHfudeRQPOYdjOnn61udHUhMXp qpo65/N8dODgYxau51BtF5woZq98LnDIEdCOIUjfa9Lt8u3jG0u8q9MU0H4r8BNScpyi j3HUGTKrdvLJvEOcl/D1qmZ5oqHen9F4D9TQB1MYxVLJV2tkbNrwzLt4BTV0xHUu2V6s STog== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=Z4Qjm9gj; spf=pass (google.com: domain of jens.wiklander@linaro.org designates 209.85.220.65 as permitted sender) smtp.mailfrom=jens.wiklander@linaro.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id u26-v6sor3340301lji.11.2018.05.22.01.09.48 for (Google Transport Security); Tue, 22 May 2018 01:09:48 -0700 (PDT) Received-SPF: pass (google.com: domain of jens.wiklander@linaro.org designates 209.85.220.65 as permitted sender) client-ip=209.85.220.65; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=Z4Qjm9gj; spf=pass (google.com: domain of jens.wiklander@linaro.org designates 209.85.220.65 as permitted sender) smtp.mailfrom=jens.wiklander@linaro.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=tyDPsf+Vtg1tzo9IFda8SAQnXbVn/CAh7JVSthamDbk=; b=Z4Qjm9gjf52YG9BpcoMRETHqyD/rGSp7DtpdP0y8L5VXHhJnD+ewRI2dfyeCr29EDQ SAr042aKSjokkz/ZVYYYYJVNChPcwMY8ukzXm+gU43+8EkIgsGXOZxET9+4suuk8Mj8v l6lHWRGAOTpxR+cUZk2sXhd1V7DHZ3RN8p8tw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=tyDPsf+Vtg1tzo9IFda8SAQnXbVn/CAh7JVSthamDbk=; b=kmcYz+7hS+93EdPZpOwQnCds31h/tJIi2hwVi9DmiIoNIovE7CuSzQ+FtIFJGeFP/Q tZgG7hZHMpM7ijYCMjNQrez1Nnk143S10mV+Gl52gerEwJcKbPkL1uO9q4XAoYBYZA1k /n0pdQnwxznAltZX2gLXP1UcHNWUWSqjAmsraglxqGKhdP+BgkNUkweU1G0/huxxEXQF vPF1Z9X6Jnmm92UKI3Ux1Ke4rRcXKamBnV7yxcVVxqhTdnLJtOXgj6FSi6jLCuGoajpG a9J6Actl+OOJO0rndVn7k7Ng7qtFpzfGUURMX1GxfECwOwVADwoJgohxn78ZheqQ7Ts6 S7rQ== X-Gm-Message-State: ALKqPweyvDZx3CffHsMmhrJ1M+2Jg6KNXWc7GzosiSztaXmNdRzh0PAa dtGHk2dyM08zSv2DPCzMHXc12xPS X-Google-Smtp-Source: AB8JxZqYum86aVuEJGvpQ/fZXn8/2+ZF6qojrpunpJESwqe0dRFq3G6m1f9bS/I7sxrOh+4g+mBltg== X-Received: by 2002:a2e:320b:: with SMTP id y11-v6mr13900016ljy.119.1526976588543; Tue, 22 May 2018 01:09:48 -0700 (PDT) Return-Path: Received: from jax.urgonet (h-84-45.A175.priv.bahnhof.se. [79.136.84.45]) by smtp.gmail.com with ESMTPSA id b5-v6sm2776168ljj.73.2018.05.22.01.09.47 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 22 May 2018 01:09:47 -0700 (PDT) From: Jens Wiklander To: linux-kernel@vger.kernel.org, Sumit Semwal , Christophe JAILLET Cc: Al Viro , Gustavo Padovan , Zhenyu Wang , Rodrigo Vivi , Christian Koenig , Laura Abbott , Jens Wiklander Subject: [PATCH] dma-buf: simplified calling conventions for dma_buf_fd() Date: Tue, 22 May 2018 10:09:25 +0200 Message-Id: <20180522080925.18158-1-jens.wiklander@linaro.org> X-Mailer: git-send-email 2.17.0 From: Al Viro Make dma_buf_fd() drop the file reference on failure, folding dma_buf_put() into it. Users of dma_buf_fd() (drm, videobuf2, ion and tee) are updated to take the new calling convention into account. Signed-off-by: Al Viro [jw: rebased and updated commit message] Signed-off-by: Jens Wiklander --- I received this patch as a response to what has become bb765d1c331f ("tee: shm: fix use-after-free via temporarily dropped reference") drivers/dma-buf/dma-buf.c | 16 +++++---- drivers/gpu/drm/drm_prime.c | 15 +++----- drivers/gpu/drm/i915/gvt/dmabuf.c | 4 +-- drivers/gpu/drm/ttm/ttm_object.c | 3 +- .../media/common/videobuf2/videobuf2-core.c | 14 ++++---- drivers/staging/android/ion/ion.c | 7 +--- drivers/tee/tee_core.c | 35 ++++++------------- drivers/tee/tee_shm.c | 8 +---- 8 files changed, 36 insertions(+), 66 deletions(-) -- 2.17.0 Acked-by: Laura Abbott diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index d78d5fc173dc..78981928758e 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -471,17 +471,21 @@ EXPORT_SYMBOL_GPL(dma_buf_export); */ int dma_buf_fd(struct dma_buf *dmabuf, int flags) { + struct file *file; int fd; - if (!dmabuf || !dmabuf->file) + if (!dmabuf) return -EINVAL; - fd = get_unused_fd_flags(flags); - if (fd < 0) - return fd; - - fd_install(fd, dmabuf->file); + file = dmabuf->file; + if (!file) + return -EINVAL; + fd = get_unused_fd_flags(flags); + if (fd >= 0) + fd_install(fd, file); + else + fput(file); return fd; } EXPORT_SYMBOL_GPL(dma_buf_fd); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 7856a9b3f8a8..06d1b7d3751a 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -662,8 +662,10 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, ret = drm_prime_add_buf_handle(&file_priv->prime, dmabuf, handle); mutex_unlock(&dev->object_name_lock); - if (ret) - goto fail_put_dmabuf; + if (unlikely(ret)) { + dma_buf_put(dmabuf); + goto out; + } out_have_handle: ret = dma_buf_fd(dmabuf, flags); @@ -673,17 +675,10 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, * and that is invariant as long as a userspace gem handle exists. * Closing the handle will clean out the cache anyway, so we don't leak. */ - if (ret < 0) { - goto fail_put_dmabuf; - } else { + if (ret >= 0) { *prime_fd = ret; ret = 0; } - - goto out; - -fail_put_dmabuf: - dma_buf_put(dmabuf); out: drm_gem_object_put_unlocked(obj); out_unlock: diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c index 6f4f8e941fc2..1783c724d96c 100644 --- a/drivers/gpu/drm/i915/gvt/dmabuf.c +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c @@ -479,7 +479,7 @@ int intel_vgpu_get_dmabuf(struct intel_vgpu *vgpu, unsigned int dmabuf_id) ret = dma_buf_fd(dmabuf, DRM_CLOEXEC | DRM_RDWR); if (ret < 0) { gvt_vgpu_err("create dma-buf fd failed ret:%d\n", ret); - goto out_free_dmabuf; + goto out_free_gem; } dmabuf_fd = ret; @@ -502,8 +502,6 @@ int intel_vgpu_get_dmabuf(struct intel_vgpu *vgpu, unsigned int dmabuf_id) return dmabuf_fd; -out_free_dmabuf: - dma_buf_put(dmabuf); out_free_gem: i915_gem_object_put(obj); out: diff --git a/drivers/gpu/drm/ttm/ttm_object.c b/drivers/gpu/drm/ttm/ttm_object.c index 1aa2baa83959..30d29d17653f 100644 --- a/drivers/gpu/drm/ttm/ttm_object.c +++ b/drivers/gpu/drm/ttm/ttm_object.c @@ -730,8 +730,7 @@ int ttm_prime_handle_to_fd(struct ttm_object_file *tfile, if (ret >= 0) { *prime_fd = ret; ret = 0; - } else - dma_buf_put(dma_buf); + } out_unref: if (base) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index d3f7bb33a54d..3baf689241cc 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -1881,15 +1881,13 @@ int vb2_core_expbuf(struct vb2_queue *q, int *fd, unsigned int type, if (ret < 0) { dprintk(3, "buffer %d, plane %d failed to export (%d)\n", index, plane, ret); - dma_buf_put(dbuf); - return ret; + } else { + dprintk(3, "buffer %d, plane %d exported as %d descriptor\n", + index, plane, ret); + *fd = ret; + ret = 0; } - - dprintk(3, "buffer %d, plane %d exported as %d descriptor\n", - index, plane, ret); - *fd = ret; - - return 0; + return ret; } EXPORT_SYMBOL_GPL(vb2_core_expbuf); diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index e74db7902549..c9fcb2a2f518 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -381,7 +381,6 @@ int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags) struct ion_buffer *buffer = NULL; struct ion_heap *heap; DEFINE_DMA_BUF_EXPORT_INFO(exp_info); - int fd; struct dma_buf *dmabuf; pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__, @@ -425,11 +424,7 @@ int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags) return PTR_ERR(dmabuf); } - fd = dma_buf_fd(dmabuf, O_CLOEXEC); - if (fd < 0) - dma_buf_put(dmabuf); - - return fd; + return dma_buf_fd(dmabuf, O_CLOEXEC); } int ion_query_heaps(struct ion_heap_query *query) diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index dd46b758852a..1cb54f6f3c6a 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -125,7 +125,6 @@ static int tee_ioctl_version(struct tee_context *ctx, static int tee_ioctl_shm_alloc(struct tee_context *ctx, struct tee_ioctl_shm_alloc_data __user *udata) { - long ret; struct tee_ioctl_shm_alloc_data data; struct tee_shm *shm; @@ -144,25 +143,18 @@ static int tee_ioctl_shm_alloc(struct tee_context *ctx, data.flags = shm->flags; data.size = shm->size; - if (copy_to_user(udata, &data, sizeof(data))) - ret = -EFAULT; - else - ret = tee_shm_get_fd(shm); + if (unlikely(copy_to_user(udata, &data, sizeof(data)))) { + tee_shm_put(shm); + return -EFAULT; + } - /* - * When user space closes the file descriptor the shared memory - * should be freed or if tee_shm_get_fd() failed then it will - * be freed immediately. - */ - tee_shm_put(shm); - return ret; + return tee_shm_get_fd(shm); } static int tee_ioctl_shm_register(struct tee_context *ctx, struct tee_ioctl_shm_register_data __user *udata) { - long ret; struct tee_ioctl_shm_register_data data; struct tee_shm *shm; @@ -182,17 +174,12 @@ tee_ioctl_shm_register(struct tee_context *ctx, data.flags = shm->flags; data.length = shm->size; - if (copy_to_user(udata, &data, sizeof(data))) - ret = -EFAULT; - else - ret = tee_shm_get_fd(shm); - /* - * When user space closes the file descriptor the shared memory - * should be freed or if tee_shm_get_fd() failed then it will - * be freed immediately. - */ - tee_shm_put(shm); - return ret; + if (unlikely(copy_to_user(udata, &data, sizeof(data)))) { + tee_shm_put(shm); + return -EFAULT; + } + + return tee_shm_get_fd(shm); } static int params_from_user(struct tee_context *ctx, struct tee_param *params, diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 07d3be6f0780..53b35b5e5776 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -355,16 +355,10 @@ EXPORT_SYMBOL_GPL(tee_shm_register); */ int tee_shm_get_fd(struct tee_shm *shm) { - int fd; - if (!(shm->flags & TEE_SHM_DMA_BUF)) return -EINVAL; - get_dma_buf(shm->dmabuf); - fd = dma_buf_fd(shm->dmabuf, O_CLOEXEC); - if (fd < 0) - dma_buf_put(shm->dmabuf); - return fd; + return dma_buf_fd(shm->dmabuf, O_CLOEXEC); } /**