stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Lars Persson <larper@axis.com>,
	Sumit Garg <sumit.garg@linaro.org>,
	Patrik Lantz <patrik.lantz@axis.com>,
	Jens Wiklander <jens.wiklander@linaro.org>
Subject: [PATCH 4.19 02/27] tee: handle lookup of shm with reference count 0
Date: Mon,  3 Jan 2022 15:23:42 +0100	[thread overview]
Message-ID: <20220103142052.248424963@linuxfoundation.org> (raw)
In-Reply-To: <20220103142052.162223000@linuxfoundation.org>

From: Jens Wiklander <jens.wiklander@linaro.org>

commit dfd0743f1d9ea76931510ed150334d571fbab49d upstream.

Since the tee subsystem does not keep a strong reference to its idle
shared memory buffers, it races with other threads that try to destroy a
shared memory through a close of its dma-buf fd or by unmapping the
memory.

In tee_shm_get_from_id() when a lookup in teedev->idr has been
successful, it is possible that the tee_shm is in the dma-buf teardown
path, but that path is blocked by the teedev mutex. Since we don't have
an API to tell if the tee_shm is in the dma-buf teardown path or not we
must find another way of detecting this condition.

Fix this by doing the reference counting directly on the tee_shm using a
new refcount_t refcount field. dma-buf is replaced by using
anon_inode_getfd() instead, this separates the life-cycle of the
underlying file from the tee_shm. tee_shm_put() is updated to hold the
mutex when decreasing the refcount to 0 and then remove the tee_shm from
teedev->idr before releasing the mutex. This means that the tee_shm can
never be found unless it has a refcount larger than 0.

Fixes: 967c9cca2cc5 ("tee: generic TEE subsystem")
Cc: stable@vger.kernel.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Lars Persson <larper@axis.com>
Reviewed-by: Sumit Garg <sumit.garg@linaro.org>
Reported-by: Patrik Lantz <patrik.lantz@axis.com>
[JW: backport to 4.19-stable]
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tee/tee_shm.c   |  177 ++++++++++++++++++------------------------------
 include/linux/tee_drv.h |    4 -
 2 files changed, 69 insertions(+), 112 deletions(-)

--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015-2016, Linaro Limited
+ * Copyright (c) 2015-2017, 2019-2021 Linaro Limited
  *
  * This software is licensed under the terms of the GNU General Public
  * License version 2, as published by the Free Software Foundation, and
@@ -11,25 +11,17 @@
  * GNU General Public License for more details.
  *
  */
+#include <linux/anon_inodes.h>
 #include <linux/device.h>
-#include <linux/dma-buf.h>
-#include <linux/fdtable.h>
 #include <linux/idr.h>
+#include <linux/mm.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/tee_drv.h>
 #include "tee_private.h"
 
-static void tee_shm_release(struct tee_shm *shm)
+static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm)
 {
-	struct tee_device *teedev = shm->teedev;
-
-	mutex_lock(&teedev->mutex);
-	idr_remove(&teedev->idr, shm->id);
-	if (shm->ctx)
-		list_del(&shm->link);
-	mutex_unlock(&teedev->mutex);
-
 	if (shm->flags & TEE_SHM_POOL) {
 		struct tee_shm_pool_mgr *poolm;
 
@@ -61,51 +53,6 @@ static void tee_shm_release(struct tee_s
 	tee_device_put(teedev);
 }
 
-static struct sg_table *tee_shm_op_map_dma_buf(struct dma_buf_attachment
-			*attach, enum dma_data_direction dir)
-{
-	return NULL;
-}
-
-static void tee_shm_op_unmap_dma_buf(struct dma_buf_attachment *attach,
-				     struct sg_table *table,
-				     enum dma_data_direction dir)
-{
-}
-
-static void tee_shm_op_release(struct dma_buf *dmabuf)
-{
-	struct tee_shm *shm = dmabuf->priv;
-
-	tee_shm_release(shm);
-}
-
-static void *tee_shm_op_map(struct dma_buf *dmabuf, unsigned long pgnum)
-{
-	return NULL;
-}
-
-static int tee_shm_op_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
-{
-	struct tee_shm *shm = dmabuf->priv;
-	size_t size = vma->vm_end - vma->vm_start;
-
-	/* Refuse sharing shared memory provided by application */
-	if (shm->flags & TEE_SHM_REGISTER)
-		return -EINVAL;
-
-	return remap_pfn_range(vma, vma->vm_start, shm->paddr >> PAGE_SHIFT,
-			       size, vma->vm_page_prot);
-}
-
-static const struct dma_buf_ops tee_shm_dma_buf_ops = {
-	.map_dma_buf = tee_shm_op_map_dma_buf,
-	.unmap_dma_buf = tee_shm_op_unmap_dma_buf,
-	.release = tee_shm_op_release,
-	.map = tee_shm_op_map,
-	.mmap = tee_shm_op_mmap,
-};
-
 static struct tee_shm *__tee_shm_alloc(struct tee_context *ctx,
 				       struct tee_device *teedev,
 				       size_t size, u32 flags)
@@ -146,6 +93,7 @@ static struct tee_shm *__tee_shm_alloc(s
 		goto err_dev_put;
 	}
 
+	refcount_set(&shm->refcount, 1);
 	shm->flags = flags | TEE_SHM_POOL;
 	shm->teedev = teedev;
 	shm->ctx = ctx;
@@ -168,21 +116,6 @@ static struct tee_shm *__tee_shm_alloc(s
 		goto err_pool_free;
 	}
 
-	if (flags & TEE_SHM_DMA_BUF) {
-		DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
-
-		exp_info.ops = &tee_shm_dma_buf_ops;
-		exp_info.size = shm->size;
-		exp_info.flags = O_RDWR;
-		exp_info.priv = shm;
-
-		shm->dmabuf = dma_buf_export(&exp_info);
-		if (IS_ERR(shm->dmabuf)) {
-			ret = ERR_CAST(shm->dmabuf);
-			goto err_rem;
-		}
-	}
-
 	if (ctx) {
 		teedev_ctx_get(ctx);
 		mutex_lock(&teedev->mutex);
@@ -191,10 +124,6 @@ static struct tee_shm *__tee_shm_alloc(s
 	}
 
 	return shm;
-err_rem:
-	mutex_lock(&teedev->mutex);
-	idr_remove(&teedev->idr, shm->id);
-	mutex_unlock(&teedev->mutex);
 err_pool_free:
 	poolm->ops->free(poolm, shm);
 err_kfree:
@@ -259,6 +188,7 @@ struct tee_shm *tee_shm_register(struct
 		goto err;
 	}
 
+	refcount_set(&shm->refcount, 1);
 	shm->flags = flags | TEE_SHM_REGISTER;
 	shm->teedev = teedev;
 	shm->ctx = ctx;
@@ -299,22 +229,6 @@ struct tee_shm *tee_shm_register(struct
 		goto err;
 	}
 
-	if (flags & TEE_SHM_DMA_BUF) {
-		DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
-
-		exp_info.ops = &tee_shm_dma_buf_ops;
-		exp_info.size = shm->size;
-		exp_info.flags = O_RDWR;
-		exp_info.priv = shm;
-
-		shm->dmabuf = dma_buf_export(&exp_info);
-		if (IS_ERR(shm->dmabuf)) {
-			ret = ERR_CAST(shm->dmabuf);
-			teedev->desc->ops->shm_unregister(ctx, shm);
-			goto err;
-		}
-	}
-
 	mutex_lock(&teedev->mutex);
 	list_add_tail(&shm->link, &ctx->list_shm);
 	mutex_unlock(&teedev->mutex);
@@ -342,6 +256,35 @@ err:
 }
 EXPORT_SYMBOL_GPL(tee_shm_register);
 
+static int tee_shm_fop_release(struct inode *inode, struct file *filp)
+{
+	tee_shm_put(filp->private_data);
+	return 0;
+}
+
+static int tee_shm_fop_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+	struct tee_shm *shm = filp->private_data;
+	size_t size = vma->vm_end - vma->vm_start;
+
+	/* Refuse sharing shared memory provided by application */
+	if (shm->flags & TEE_SHM_USER_MAPPED)
+		return -EINVAL;
+
+	/* check for overflowing the buffer's size */
+	if (vma->vm_pgoff + vma_pages(vma) > shm->size >> PAGE_SHIFT)
+		return -EINVAL;
+
+	return remap_pfn_range(vma, vma->vm_start, shm->paddr >> PAGE_SHIFT,
+			       size, vma->vm_page_prot);
+}
+
+static const struct file_operations tee_shm_fops = {
+	.owner = THIS_MODULE,
+	.release = tee_shm_fop_release,
+	.mmap = tee_shm_fop_mmap,
+};
+
 /**
  * tee_shm_get_fd() - Increase reference count and return file descriptor
  * @shm:	Shared memory handle
@@ -354,10 +297,11 @@ int tee_shm_get_fd(struct tee_shm *shm)
 	if (!(shm->flags & TEE_SHM_DMA_BUF))
 		return -EINVAL;
 
-	get_dma_buf(shm->dmabuf);
-	fd = dma_buf_fd(shm->dmabuf, O_CLOEXEC);
+	/* matched by tee_shm_put() in tee_shm_op_release() */
+	refcount_inc(&shm->refcount);
+	fd = anon_inode_getfd("tee_shm", &tee_shm_fops, shm, O_RDWR);
 	if (fd < 0)
-		dma_buf_put(shm->dmabuf);
+		tee_shm_put(shm);
 	return fd;
 }
 
@@ -367,17 +311,7 @@ int tee_shm_get_fd(struct tee_shm *shm)
  */
 void tee_shm_free(struct tee_shm *shm)
 {
-	/*
-	 * dma_buf_put() decreases the dmabuf reference counter and will
-	 * call tee_shm_release() when the last reference is gone.
-	 *
-	 * In the case of driver private memory we call tee_shm_release
-	 * directly instead as it doesn't have a reference counter.
-	 */
-	if (shm->flags & TEE_SHM_DMA_BUF)
-		dma_buf_put(shm->dmabuf);
-	else
-		tee_shm_release(shm);
+	tee_shm_put(shm);
 }
 EXPORT_SYMBOL_GPL(tee_shm_free);
 
@@ -484,10 +418,15 @@ struct tee_shm *tee_shm_get_from_id(stru
 	teedev = ctx->teedev;
 	mutex_lock(&teedev->mutex);
 	shm = idr_find(&teedev->idr, id);
+	/*
+	 * If the tee_shm was found in the IDR it must have a refcount
+	 * larger than 0 due to the guarantee in tee_shm_put() below. So
+	 * it's safe to use refcount_inc().
+	 */
 	if (!shm || shm->ctx != ctx)
 		shm = ERR_PTR(-EINVAL);
-	else if (shm->flags & TEE_SHM_DMA_BUF)
-		get_dma_buf(shm->dmabuf);
+	else
+		refcount_inc(&shm->refcount);
 	mutex_unlock(&teedev->mutex);
 	return shm;
 }
@@ -499,7 +438,25 @@ EXPORT_SYMBOL_GPL(tee_shm_get_from_id);
  */
 void tee_shm_put(struct tee_shm *shm)
 {
-	if (shm->flags & TEE_SHM_DMA_BUF)
-		dma_buf_put(shm->dmabuf);
+	struct tee_device *teedev = shm->teedev;
+	bool do_release = false;
+
+	mutex_lock(&teedev->mutex);
+	if (refcount_dec_and_test(&shm->refcount)) {
+		/*
+		 * refcount has reached 0, we must now remove it from the
+		 * IDR before releasing the mutex. This will guarantee that
+		 * the refcount_inc() in tee_shm_get_from_id() never starts
+		 * from 0.
+		 */
+		idr_remove(&teedev->idr, shm->id);
+		if (shm->ctx)
+			list_del(&shm->link);
+		do_release = true;
+	}
+	mutex_unlock(&teedev->mutex);
+
+	if (do_release)
+		tee_shm_release(teedev, shm);
 }
 EXPORT_SYMBOL_GPL(tee_shm_put);
--- a/include/linux/tee_drv.h
+++ b/include/linux/tee_drv.h
@@ -177,7 +177,7 @@ void tee_device_unregister(struct tee_de
  * @offset:	offset of buffer in user space
  * @pages:	locked pages from userspace
  * @num_pages:	number of locked pages
- * @dmabuf:	dmabuf used to for exporting to user space
+ * @refcount:	reference counter
  * @flags:	defined by TEE_SHM_* in tee_drv.h
  * @id:		unique id of a shared memory object on this device
  *
@@ -194,7 +194,7 @@ struct tee_shm {
 	unsigned int offset;
 	struct page **pages;
 	size_t num_pages;
-	struct dma_buf *dmabuf;
+	refcount_t refcount;
 	u32 flags;
 	int id;
 };



  parent reply	other threads:[~2022-01-03 14:25 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-03 14:23 [PATCH 4.19 00/27] 4.19.224-rc1 review Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 01/27] HID: asus: Add depends on USB_HID to HID_ASUS Kconfig option Greg Kroah-Hartman
2022-01-03 14:23 ` Greg Kroah-Hartman [this message]
2022-01-03 14:23 ` [PATCH 4.19 03/27] Input: i8042 - add deferred probe support Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 04/27] Input: i8042 - enable deferred probe quirk for ASUS UM325UA Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 05/27] platform/x86: apple-gmux: use resource_size() with res Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 06/27] recordmcount.pl: fix typo in s390 mcount regex Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 07/27] selinux: initialize proto variable in selinux_ip_postroute_compat() Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 08/27] scsi: lpfc: Terminate string in lpfc_debugfs_nvmeio_trc_write() Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 09/27] udp: using datalen to cap ipv6 udp max gso segments Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 10/27] selftests: Calculate udpgso segment count without header adjustment Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 11/27] sctp: use call_rcu to free endpoint Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 12/27] net: usb: pegasus: Do not drop long Ethernet frames Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 13/27] NFC: st21nfca: Fix memory leak in device probe and remove Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 14/27] net/mlx5e: Fix wrong features assignment in case of error Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 15/27] selftests/net: udpgso_bench_tx: fix dst ip argument Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 16/27] fsl/fman: Fix missing put_device() call in fman_port_probe Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 17/27] i2c: validate user data in compat ioctl Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 18/27] nfc: uapi: use kernel size_t to fix user-space builds Greg Kroah-Hartman
2022-01-03 14:23 ` [PATCH 4.19 19/27] uapi: fix linux/nfc.h userspace compilation errors Greg Kroah-Hartman
2022-01-03 14:24 ` [PATCH 4.19 20/27] xhci: Fresco FL1100 controller should not have BROKEN_MSI quirk set Greg Kroah-Hartman
2022-01-03 14:24 ` [PATCH 4.19 21/27] usb: gadget: f_fs: Clear ffs_eventfd in ffs_data_clear Greg Kroah-Hartman
2022-01-03 14:24 ` [PATCH 4.19 22/27] usb: mtu3: set interval of FS intr and isoc endpoint Greg Kroah-Hartman
2022-01-03 14:24 ` [PATCH 4.19 23/27] binder: fix async_free_space accounting for empty parcels Greg Kroah-Hartman
2022-01-03 14:24 ` [PATCH 4.19 24/27] scsi: vmw_pvscsi: Set residual data length conditionally Greg Kroah-Hartman
2022-01-03 14:24 ` [PATCH 4.19 25/27] Input: appletouch - initialize work before device registration Greg Kroah-Hartman
2022-01-03 14:24 ` [PATCH 4.19 26/27] Input: spaceball - fix parsing of movement data packets Greg Kroah-Hartman
2022-01-03 14:24 ` [PATCH 4.19 27/27] net: fix use-after-free in tw_timer_handler Greg Kroah-Hartman
2022-01-03 20:18 ` [PATCH 4.19 00/27] 4.19.224-rc1 review Pavel Machek
2022-01-04  1:25 ` Guenter Roeck
2022-01-04  9:53 ` Jon Hunter
2022-01-04 13:00 ` Naresh Kamboju
2022-01-04 15:51 ` Sudip Mukherjee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220103142052.248424963@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=jens.wiklander@linaro.org \
    --cc=larper@axis.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patrik.lantz@axis.com \
    --cc=stable@vger.kernel.org \
    --cc=sumit.garg@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).