Tee-dev February 2020

tee-dev@lists.linaro.org

5 participants
6 discussions

[PATCH] tee: amdtee: out of bounds read in find_session()

by Dan Carpenter

The "index" is a user provided value from 0-USHRT_MAX. If it's over TEE_NUM_SESSIONS (31) then it results in an out of bounds read when we call test_bit(index, sess->sess_mask). Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver") Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> --- drivers/tee/amdtee/core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c index 6370bb55f512..dbc238c7c263 100644 --- a/drivers/tee/amdtee/core.c +++ b/drivers/tee/amdtee/core.c @@ -139,6 +139,9 @@ static struct amdtee_session *find_session(struct amdtee_context_data *ctxdata, u32 index = get_session_index(session); struct amdtee_session *sess; + if (index >= TEE_NUM_SESSIONS) + return NULL; + list_for_each_entry(sess, &ctxdata->sess_list, list_node) if (ta_handle == sess->ta_handle && test_bit(index, sess->sess_mask)) -- 2.11.0

4 years, 8 months

[GIT PULL] TEE shared memory cleanup for v5.7

by Jens Wiklander

Hello arm-soc maintainers, Please pull these cleanup patches for shared memory in the TEE subsystem. Thanks, Jens The following changes since commit 11a48a5a18c63fd7621bb050228cebf13566e4d8: Linux 5.6-rc2 (2020-02-16 13:16:59 -0800) are available in the Git repository at: https://git.linaro.org/people/jens.wiklander/linux-tee.git tags/tee-cleanup-for-5.7 for you to fetch changes up to 758ecf13a41a9dc4f019c1381566132ef46c08ee: tee: tee_shm_op_mmap(): use TEE_SHM_USER_MAPPED (2020-02-28 13:37:42 +0100) ---------------------------------------------------------------- Cleanup shared memory handing in TEE subsystem The highlights are: - Removing redundant or unused fields in struct tee_shm - Only assign userspace shm IDs for shared memory objects originating from user space ---------------------------------------------------------------- Jens Wiklander (5): tee: remove linked list of struct tee_shm tee: remove unused tee_shm_priv_alloc() tee: don't assign shm id for private shms tee: remove redundant teedev in struct tee_shm tee: tee_shm_op_mmap(): use TEE_SHM_USER_MAPPED drivers/tee/tee_core.c | 1 - drivers/tee/tee_private.h | 3 +- drivers/tee/tee_shm.c | 85 +++++++++++++---------------------------------- include/linux/tee_drv.h | 19 +---------- 4 files changed, 27 insertions(+), 81 deletions(-)

4 years, 9 months

out of memory failures in xtest due to tee_shm_free() from Linux not unregistering SHM DMA buffer in kexec path

by Rajesh Ravi

Hi, As SHM DMA memory is not unregistered by tee_shm_free() Linux tee driver API in kexec path (Case A), we 're facing lot of issues including failures in OP-TEE xtest. Can any one of you suggest the solution. *Problem Description* Case A If tee_shm_free() is called in shutdown() of Linux tee_client_driver and kexec kernel is booted, shutdown() & tee_shm_free() are invoked but tee_shm_release() is not invoked on DMA SHM buffer. Case B If tee_shm_free() is called on SHM DMA buffer previously allocated from rmmod path[module_exit()] It unregisters the SHM memory and sends OPTEE_MSG_CMD_UNREGISTER_SHM to optee_os through optee_shm_unregister(). *Call Sequence* Case A: kexec path *.shutdown()-->tee_shm_free()-->dma_buf_put()* Case B: rmmod path *.shutdown()-->tee_shm_free()-->tee_shm_release()-->optee_shm_unregister() -->optee_do_call_with_arg() [cmd = OPTEE_MSG_CMD_UNREGISTER_SHM]* *Repercussions of the issue: xtest failure due to out of memory* If we register a big buffer of say 8MB in Linux tee client driver, if the same memory is not unregistered, it can cause overhead of 2*1024*8 = 16KB memory overhead for shm page book keeping data structures calloc'd in optee_os. After kexec it causes 16 +16=32KB which significant memory on a minimal heap of size, say 64KB. This causes failures with asymmetric crypto operations of xtest due to out of memory error. *Context* In Linux kernel tee_client_driver probe() we 're calling tee_shm_alloc() with flags=TEE_SHM_MAPPED | TEE_SHM_DMA_BUF In remove() & shutdown() functions of the driver: we 're calling tee_shm_free() on shm reference allocated in probe. -- Thanks & Regards, Rajesh

4 years, 9 months

[GIT PULL] amdtee driver fix for v5.6

by Jens Wiklander

Hello arm-soc maintainers, Please pull this AMDTEE driver fix for a memory leak in one of the error paths of amdtee_open_session() Thanks, Jens The following changes since commit 11a48a5a18c63fd7621bb050228cebf13566e4d8: Linux 5.6-rc2 (2020-02-16 13:16:59 -0800) are available in the Git repository at: https://git.linaro.org/people/jens.wiklander/linux-tee.git tags/tee-amdtee-fix-for-5.6 for you to fetch changes up to b83685bceedbeed33a6adc2d0579a011708d2b18: tee: amdtee: fix memory leak in amdtee_open_session() (2020-02-27 16:22:05 +0100) ---------------------------------------------------------------- Fix AMDTEE memory leak in amdtee_open_session() ---------------------------------------------------------------- Dan Carpenter (1): tee: amdtee: fix memory leak in amdtee_open_session() drivers/tee/amdtee/core.c | 48 +++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-)

4 years, 9 months

[PATCH] tee: amdtee: Fix error handling in amdtee_open_session()

by Dan Carpenter

On these error paths the "sess" variable isn't freed. It's a refcounted pointer so we need to call kref_put(). The other problem is that the error code isn't always set. I re-arranged the code a bit so the error case is always handled before the success case and the error paths are indented two tabs. Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver") Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> --- drivers/tee/amdtee/core.c | 52 +++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c index 6370bb55f512..40f3ff4cfae6 100644 --- a/drivers/tee/amdtee/core.c +++ b/drivers/tee/amdtee/core.c @@ -212,6 +212,19 @@ static int copy_ta_binary(struct tee_context *ctx, void *ptr, void **ta, return rc; } +static void destroy_session(struct kref *ref) +{ + struct amdtee_session *sess = container_of(ref, struct amdtee_session, + refcount); + + /* Unload the TA from TEE */ + handle_unload_ta(sess->ta_handle); + mutex_lock(&session_list_mutex); + list_del(&sess->list_node); + mutex_unlock(&session_list_mutex); + kfree(sess); +} + int amdtee_open_session(struct tee_context *ctx, struct tee_ioctl_open_session_arg *arg, struct tee_param *param) @@ -236,14 +249,14 @@ int amdtee_open_session(struct tee_context *ctx, /* Load the TA binary into TEE environment */ handle_load_ta(ta, ta_size, arg); - if (arg->ret == TEEC_SUCCESS) { - mutex_lock(&session_list_mutex); - sess = alloc_session(ctxdata, arg->session); - mutex_unlock(&session_list_mutex); + if (arg->ret != TEEC_SUCCESS) { + rc = -EINVAL; + goto out; } - if (arg->ret != TEEC_SUCCESS) - goto out; + mutex_lock(&session_list_mutex); + sess = alloc_session(ctxdata, arg->session); + mutex_unlock(&session_list_mutex); if (!sess) { rc = -ENOMEM; @@ -259,40 +272,31 @@ int amdtee_open_session(struct tee_context *ctx, if (i >= TEE_NUM_SESSIONS) { pr_err("reached maximum session count %d\n", TEE_NUM_SESSIONS); + kref_put(&sess->refcount, destroy_session); rc = -ENOMEM; goto out; } /* Open session with loaded TA */ handle_open_session(arg, &session_info, param); - - if (arg->ret == TEEC_SUCCESS) { - sess->session_info[i] = session_info; - set_session_id(sess->ta_handle, i, &arg->session); - } else { + if (arg->ret != TEEC_SUCCESS) { pr_err("open_session failed %d\n", arg->ret); spin_lock(&sess->lock); clear_bit(i, sess->sess_mask); spin_unlock(&sess->lock); + kref_put(&sess->refcount, destroy_session); + rc = -EINVAL; + goto out; } + + sess->session_info[i] = session_info; + set_session_id(sess->ta_handle, i, &arg->session); + out: free_pages((u64)ta, get_order(ta_size)); return rc; } -static void destroy_session(struct kref *ref) -{ - struct amdtee_session *sess = container_of(ref, struct amdtee_session, - refcount); - - /* Unload the TA from TEE */ - handle_unload_ta(sess->ta_handle); - mutex_lock(&session_list_mutex); - list_del(&sess->list_node); - mutex_unlock(&session_list_mutex); - kfree(sess); -} - int amdtee_close_session(struct tee_context *ctx, u32 session) { struct amdtee_context_data *ctxdata = ctx->data; -- 2.11.0

4 years, 9 months

[PATCH RESEND -next] tee: amdtee: amdtee depends on CRYPTO_DEV_CCP_DD

by Hongbo Yao

If CRYPTO_DEV_CCP_DD=m and AMDTEE=y, the following error is seen while building call.c or core.c drivers/tee/amdtee/call.o: In function `handle_unload_ta': call.c:(.text+0x35f): undefined reference to `psp_tee_process_cmd' drivers/tee/amdtee/core.o: In function `amdtee_driver_init': core.c:(.init.text+0xf): undefined reference to `psp_check_tee_status Fix the config dependency for AMDTEE here. Reported-by: Hulk Robot <hulkci(a)huawei.com> Fixes: 757cc3e9ff ("tee: add AMD-TEE driver") Signed-off-by: Hongbo Yao <yaohongbo(a)huawei.com> Reviewed-by: Rijo Thomas <Rijo-john.Thomas(a)amd.com> --- drivers/tee/amdtee/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tee/amdtee/Kconfig b/drivers/tee/amdtee/Kconfig index 4e32b6413b41..191f9715fa9a 100644 --- a/drivers/tee/amdtee/Kconfig +++ b/drivers/tee/amdtee/Kconfig @@ -3,6 +3,6 @@ config AMDTEE tristate "AMD-TEE" default m - depends on CRYPTO_DEV_SP_PSP + depends on CRYPTO_DEV_SP_PSP && CRYPTO_DEV_CCP_DD help This implements AMD's Trusted Execution Environment (TEE) driver. -- 2.20.1

4 years, 9 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

Tee-dev February 2020