| From dfd0743f1d9ea76931510ed150334d571fbab49d Mon Sep 17 00:00:00 2001 |
| From: Jens Wiklander <jens.wiklander@linaro.org> |
| Date: Thu, 9 Dec 2021 15:59:37 +0100 |
| Subject: tee: handle lookup of shm with reference count 0 |
| |
| 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; |
| }; |