From 95ffe4ca43877eea176d7e95aa0d38bbdc3d2903 Mon Sep 17 00:00:00 2001 From: Jens Wiklander Date: Thu, 28 Dec 2017 10:08:00 +0100 Subject: tee: add start argument to shm_register callback Adds a start argument to the shm_register callback to allow the callback to check memory type of the passed pages. Signed-off-by: Jens Wiklander --- drivers/tee/optee/call.c | 6 ++++-- drivers/tee/optee/optee_private.h | 6 ++++-- drivers/tee/tee_shm.c | 2 +- include/linux/tee_drv.h | 3 ++- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c index e675e82ff095..d61c14b788f2 100644 --- a/drivers/tee/optee/call.c +++ b/drivers/tee/optee/call.c @@ -536,7 +536,8 @@ void optee_free_pages_list(void *list, size_t num_entries) } int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm, - struct page **pages, size_t num_pages) + struct page **pages, size_t num_pages, + unsigned long start) { struct tee_shm *shm_arg = NULL; struct optee_msg_arg *msg_arg; @@ -606,7 +607,8 @@ int optee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm) } int optee_shm_register_supp(struct tee_context *ctx, struct tee_shm *shm, - struct page **pages, size_t num_pages) + struct page **pages, size_t num_pages, + unsigned long start) { /* * We don't want to register supplicant memory in OP-TEE. diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index de7962ebc1b6..f04930879762 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -173,11 +173,13 @@ void optee_enable_shm_cache(struct optee *optee); void optee_disable_shm_cache(struct optee *optee); int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm, - struct page **pages, size_t num_pages); + struct page **pages, size_t num_pages, + unsigned long start); int optee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm); int optee_shm_register_supp(struct tee_context *ctx, struct tee_shm *shm, - struct page **pages, size_t num_pages); + struct page **pages, size_t num_pages, + unsigned long start); int optee_shm_unregister_supp(struct tee_context *ctx, struct tee_shm *shm); int optee_from_msg_param(struct tee_param *params, size_t num_params, diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 04e1b8b37046..6a17b02ada5e 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -299,7 +299,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr, } rc = teedev->desc->ops->shm_register(ctx, shm, shm->pages, - shm->num_pages); + shm->num_pages, start); if (rc) { ret = ERR_PTR(rc); goto err; diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h index a1d7f467657c..230a1ebbf3bc 100644 --- a/include/linux/tee_drv.h +++ b/include/linux/tee_drv.h @@ -108,7 +108,8 @@ struct tee_driver_ops { int (*supp_send)(struct tee_context *ctx, u32 ret, u32 num_params, struct tee_param *param); int (*shm_register)(struct tee_context *ctx, struct tee_shm *shm, - struct page **pages, size_t num_pages); + struct page **pages, size_t num_pages, + unsigned long start); int (*shm_unregister)(struct tee_context *ctx, struct tee_shm *shm); }; -- cgit v1.2.3-58-ga151 From cdbcf83d29c1bf2aaa65260e74beaac1bcdc231c Mon Sep 17 00:00:00 2001 From: Jens Wiklander Date: Thu, 28 Dec 2017 11:14:05 +0100 Subject: tee: optee: check type of registered shared memory Checks the memory type of the pages to be registered as shared memory. Only normal cached memory is allowed. Signed-off-by: Jens Wiklander --- drivers/tee/optee/call.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c index d61c14b788f2..47b12b7fd02d 100644 --- a/drivers/tee/optee/call.c +++ b/drivers/tee/optee/call.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -535,6 +536,41 @@ void optee_free_pages_list(void *list, size_t num_entries) free_pages_exact(list, get_pages_list_size(num_entries)); } +static bool is_normal_memory(pgprot_t p) +{ +#if defined(CONFIG_ARM) + return (pgprot_val(p) & L_PTE_MT_MASK) == L_PTE_MT_WRITEALLOC; +#elif defined(CONFIG_ARM64) + return (pgprot_val(p) & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL); +#else +#error "Unuspported architecture" +#endif +} + +static int __check_mem_type(struct vm_area_struct *vma, unsigned long end) +{ + while (vma && is_normal_memory(vma->vm_page_prot)) { + if (vma->vm_end >= end) + return 0; + vma = vma->vm_next; + } + + return -EINVAL; +} + +static int check_mem_type(unsigned long start, size_t num_pages) +{ + struct mm_struct *mm = current->mm; + int rc; + + down_read(&mm->mmap_sem); + rc = __check_mem_type(find_vma(mm, start), + start + num_pages * PAGE_SIZE); + up_read(&mm->mmap_sem); + + return rc; +} + int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm, struct page **pages, size_t num_pages, unsigned long start) @@ -543,11 +579,15 @@ int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm, struct optee_msg_arg *msg_arg; u64 *pages_list; phys_addr_t msg_parg; - int rc = 0; + int rc; if (!num_pages) return -EINVAL; + rc = check_mem_type(start, num_pages); + if (rc) + return rc; + pages_list = optee_allocate_pages_list(num_pages); if (!pages_list) return -ENOMEM; @@ -614,7 +654,7 @@ int optee_shm_register_supp(struct tee_context *ctx, struct tee_shm *shm, * We don't want to register supplicant memory in OP-TEE. * Instead information about it will be passed in RPC code. */ - return 0; + return check_mem_type(start, num_pages); } int optee_shm_unregister_supp(struct tee_context *ctx, struct tee_shm *shm) -- cgit v1.2.3-58-ga151 From 80ec6f5de60b6934f145b2f7e5369592bcab85f3 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Fri, 22 Dec 2017 17:01:22 +0000 Subject: tee: shm: make function __tee_shm_alloc static The function __tee_shm_alloc is local to the source and does not need to be in global scope, so make it static. Cleans up sparse warning: symbol '__tee_shm_alloc' was not declared. Should it be static? Signed-off-by: Colin Ian King Signed-off-by: Jens Wiklander --- drivers/tee/tee_shm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 6a17b02ada5e..511eb298949b 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -112,9 +112,9 @@ static const struct dma_buf_ops tee_shm_dma_buf_ops = { .mmap = tee_shm_op_mmap, }; -struct tee_shm *__tee_shm_alloc(struct tee_context *ctx, - struct tee_device *teedev, - size_t size, u32 flags) +static struct tee_shm *__tee_shm_alloc(struct tee_context *ctx, + struct tee_device *teedev, + size_t size, u32 flags) { struct tee_shm_pool_mgr *poolm = NULL; struct tee_shm *shm; -- cgit v1.2.3-58-ga151 From c94f31b526fe658c25dd2d07c90486a85437f01c Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Fri, 22 Dec 2017 17:51:50 +0000 Subject: tee: shm: don't put_page on null shm->pages In the case that shm->pages fails to allocate, the current exit error path will try to put_page on a null shm->pages and cause a null pointer dereference when accessing shm->pages[n]. Fix this by only performing the put_page and kfree on shm->pages if it is not null. Detected by CoverityScan, CID#1463283 ("Dereference after null check") Fixes: 033ddf12bcf5 ("tee: add register user memory") Signed-off-by: Colin Ian King Signed-off-by: Jens Wiklander --- drivers/tee/tee_shm.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 511eb298949b..6f36da9ee412 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -335,9 +335,11 @@ err: idr_remove(&teedev->idr, shm->id); mutex_unlock(&teedev->mutex); } - for (n = 0; n < shm->num_pages; n++) - put_page(shm->pages[n]); - kfree(shm->pages); + if (shm->pages) { + for (n = 0; n < shm->num_pages; n++) + put_page(shm->pages[n]); + kfree(shm->pages); + } } kfree(shm); teedev_ctx_put(ctx); -- cgit v1.2.3-58-ga151 From 2490cdf6435b1d3cac0dbf710cd752487c67c296 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sat, 6 Jan 2018 12:22:30 +0300 Subject: tee: shm: Potential NULL dereference calling tee_shm_register() get_user_pages_fast() can return zero in certain error paths. We should handle that or else it means we accidentally return ERR_PTR(0) which is NULL instead of an error pointer. The callers are not expecting that and will crash with a NULL dereference. Fixes: 033ddf12bcf5 ("tee: add register user memory") Signed-off-by: Dan Carpenter Signed-off-by: Jens Wiklander --- drivers/tee/tee_shm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 6f36da9ee412..556960a1bab3 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -283,7 +283,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr, if (rc > 0) shm->num_pages = rc; if (rc != num_pages) { - if (rc > 0) + if (rc >= 0) rc = -ENOMEM; ret = ERR_PTR(rc); goto err; -- cgit v1.2.3-58-ga151