This patch series addresses the bug report submitted by Dan Carpenter.
Link: https://lists.linaro.org/pipermail/tee-dev/2020-January/001417.html
Since, these patches are based on cryptodev-2.6 tree, I have included linux-crypto list as well.
This patch series does not fix the static checker warning reported due to incorrect use of IS_ERR. Colin Ian King has submitted a fix for this issue. Link: https://lkml.org/lkml/2020/1/8/88
Rijo Thomas (5): tee: amdtee: remove unused variable initialization tee: amdtee: print error message if tee not present tee: amdtee: skip tee_device_unregister if tee_device_alloc fails tee: amdtee: rename err label to err_device_unregister tee: amdtee: remove redundant NULL check for pool
drivers/tee/amdtee/call.c | 14 +++++++------- drivers/tee/amdtee/core.c | 32 +++++++++++++++++--------------- 2 files changed, 24 insertions(+), 22 deletions(-)
-- 2.17.1
Remove unused variable initialization from driver code.
If enabled as a compiler option, compiler may throw warning for unused assignments.
Reported-by: Dan Carpenter dan.carpenter@oracle.com Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver") Signed-off-by: Rijo Thomas Rijo-john.Thomas@amd.com --- drivers/tee/amdtee/call.c | 14 +++++++------- drivers/tee/amdtee/core.c | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/tee/amdtee/call.c b/drivers/tee/amdtee/call.c index 87ccad256686..096dd4d92d39 100644 --- a/drivers/tee/amdtee/call.c +++ b/drivers/tee/amdtee/call.c @@ -124,8 +124,8 @@ static int amd_params_to_tee_params(struct tee_param *tee, u32 count, int handle_unload_ta(u32 ta_handle) { struct tee_cmd_unload_ta cmd = {0}; - int ret = 0; u32 status; + int ret;
if (!ta_handle) return -EINVAL; @@ -145,8 +145,8 @@ int handle_unload_ta(u32 ta_handle) int handle_close_session(u32 ta_handle, u32 info) { struct tee_cmd_close_session cmd = {0}; - int ret = 0; u32 status; + int ret;
if (ta_handle == 0) return -EINVAL; @@ -167,8 +167,8 @@ int handle_close_session(u32 ta_handle, u32 info) void handle_unmap_shmem(u32 buf_id) { struct tee_cmd_unmap_shared_mem cmd = {0}; - int ret = 0; u32 status; + int ret;
cmd.buf_id = buf_id;
@@ -183,7 +183,7 @@ int handle_invoke_cmd(struct tee_ioctl_invoke_arg *arg, u32 sinfo, struct tee_param *p) { struct tee_cmd_invoke_cmd cmd = {0}; - int ret = 0; + int ret;
if (!arg || (!p && arg->num_params)) return -EINVAL; @@ -229,7 +229,7 @@ int handle_map_shmem(u32 count, struct shmem_desc *start, u32 *buf_id) { struct tee_cmd_map_shared_mem *cmd; phys_addr_t paddr; - int ret = 0, i; + int ret, i; u32 status;
if (!count || !start || !buf_id) @@ -294,7 +294,7 @@ int handle_open_session(struct tee_ioctl_open_session_arg *arg, u32 *info, struct tee_param *p) { struct tee_cmd_open_session cmd = {0}; - int ret = 0; + int ret;
if (!arg || !info || (!p && arg->num_params)) return -EINVAL; @@ -342,7 +342,7 @@ int handle_load_ta(void *data, u32 size, struct tee_ioctl_open_session_arg *arg) { struct tee_cmd_load_ta cmd = {0}; phys_addr_t blob; - int ret = 0; + int ret;
if (size == 0 || !data || !arg) return -EINVAL; diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c index 9d0cee1c837f..b3f8235579f7 100644 --- a/drivers/tee/amdtee/core.c +++ b/drivers/tee/amdtee/core.c @@ -50,7 +50,7 @@ static int amdtee_open(struct tee_context *ctx)
static void release_session(struct amdtee_session *sess) { - int i = 0; + int i;
/* Close any open session */ for (i = 0; i < TEE_NUM_SESSIONS; ++i) { @@ -173,7 +173,7 @@ static int copy_ta_binary(struct tee_context *ctx, void *ptr, void **ta, u16 hi_ver; u8 seq_n[8]; } *uuid = ptr; - int n = 0, rc = 0; + int n, rc = 0;
n = snprintf(fw_name, TA_PATH_MAX, "%s/%08x-%04x-%04x-%02x%02x%02x%02x%02x%02x%02x%02x.bin", @@ -219,9 +219,9 @@ int amdtee_open_session(struct tee_context *ctx, struct amdtee_context_data *ctxdata = ctx->data; struct amdtee_session *sess = NULL; u32 session_info; - void *ta = NULL; size_t ta_size; - int rc = 0, i; + int rc, i; + void *ta;
if (arg->clnt_login != TEE_IOCTL_LOGIN_PUBLIC) { pr_err("unsupported client login method\n"); @@ -368,8 +368,8 @@ int amdtee_map_shmem(struct tee_shm *shm)
void amdtee_unmap_shmem(struct tee_shm *shm) { + struct amdtee_shm_data *shmnode; u32 buf_id; - struct amdtee_shm_data *shmnode = NULL;
if (!shm) return; @@ -434,9 +434,9 @@ static const struct tee_desc amdtee_desc = {
static int __init amdtee_driver_init(void) { - struct amdtee *amdtee = NULL; struct tee_device *teedev; - struct tee_shm_pool *pool = ERR_PTR(-EINVAL); + struct tee_shm_pool *pool; + struct amdtee *amdtee; int rc;
rc = psp_check_tee_status();
If there is no TEE with which the driver can communicate, then print an error message and return.
Suggested-by: Dan Carpenter dan.carpenter@oracle.com Signed-off-by: Rijo Thomas Rijo-john.Thomas@amd.com --- drivers/tee/amdtee/core.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c index b3f8235579f7..0840be03a3ab 100644 --- a/drivers/tee/amdtee/core.c +++ b/drivers/tee/amdtee/core.c @@ -440,8 +440,10 @@ static int __init amdtee_driver_init(void) int rc;
rc = psp_check_tee_status(); - if (rc) - goto err_fail; + if (rc) { + pr_err("amd-tee driver: tee not present\n"); + return rc; + }
drv_data = kzalloc(sizeof(*drv_data), GFP_KERNEL); if (IS_ERR(drv_data)) @@ -490,7 +492,6 @@ static int __init amdtee_driver_init(void) kfree(drv_data); drv_data = NULL;
-err_fail: pr_err("amd-tee driver initialization failed\n"); return rc; }
Currently, if tee_device_alloc() fails, then tee_device_unregister() is a no-op. Therefore, skip the function call to tee_device_unregister() by introducing a new goto label 'err_free_pool'.
Reported-by: Dan Carpenter dan.carpenter@oracle.com Signed-off-by: Rijo Thomas Rijo-john.Thomas@amd.com --- drivers/tee/amdtee/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c index 0840be03a3ab..c657b9728eae 100644 --- a/drivers/tee/amdtee/core.c +++ b/drivers/tee/amdtee/core.c @@ -465,7 +465,7 @@ static int __init amdtee_driver_init(void) teedev = tee_device_alloc(&amdtee_desc, NULL, pool, amdtee); if (IS_ERR(teedev)) { rc = PTR_ERR(teedev); - goto err; + goto err_free_pool; } amdtee->teedev = teedev;
@@ -482,6 +482,8 @@ static int __init amdtee_driver_init(void)
err: tee_device_unregister(amdtee->teedev); + +err_free_pool: if (pool) tee_shm_pool_free(pool);
Rename err label to err_device_unregister for better readability.
Suggested-by: Dan Carpenter dan.carpenter@oracle.com Signed-off-by: Rijo Thomas Rijo-john.Thomas@amd.com --- drivers/tee/amdtee/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c index c657b9728eae..45402844b669 100644 --- a/drivers/tee/amdtee/core.c +++ b/drivers/tee/amdtee/core.c @@ -471,7 +471,7 @@ static int __init amdtee_driver_init(void)
rc = tee_device_register(amdtee->teedev); if (rc) - goto err; + goto err_device_unregister;
amdtee->pool = pool;
@@ -480,7 +480,7 @@ static int __init amdtee_driver_init(void) pr_info("amd-tee driver initialization successful\n"); return 0;
-err: +err_device_unregister: tee_device_unregister(amdtee->teedev);
err_free_pool:
Remove NULL check for pool variable, since in the current code path it is guaranteed to be non-NULL.
Reported-by: Dan Carpenter dan.carpenter@oracle.com Signed-off-by: Rijo Thomas Rijo-john.Thomas@amd.com --- drivers/tee/amdtee/core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c index 45402844b669..be8937eb5d43 100644 --- a/drivers/tee/amdtee/core.c +++ b/drivers/tee/amdtee/core.c @@ -484,8 +484,7 @@ static int __init amdtee_driver_init(void) tee_device_unregister(amdtee->teedev);
err_free_pool: - if (pool) - tee_shm_pool_free(pool); + tee_shm_pool_free(pool);
err_kfree_amdtee: kfree(amdtee);
On Thu, Jan 09, 2020 at 06:23:17PM +0530, Rijo Thomas wrote:
This patch series addresses the bug report submitted by Dan Carpenter.
Link: https://lists.linaro.org/pipermail/tee-dev/2020-January/001417.html
Since, these patches are based on cryptodev-2.6 tree, I have included linux-crypto list as well.
This patch series does not fix the static checker warning reported due to incorrect use of IS_ERR. Colin Ian King has submitted a fix for this issue. Link: https://lkml.org/lkml/2020/1/8/88
Rijo Thomas (5): tee: amdtee: remove unused variable initialization tee: amdtee: print error message if tee not present tee: amdtee: skip tee_device_unregister if tee_device_alloc fails tee: amdtee: rename err label to err_device_unregister tee: amdtee: remove redundant NULL check for pool
drivers/tee/amdtee/call.c | 14 +++++++------- drivers/tee/amdtee/core.c | 32 +++++++++++++++++--------------- 2 files changed, 24 insertions(+), 22 deletions(-)
All applied. Thanks.
-
Dan Carpenter -
Herbert Xu -
Rijo Thomas