diff options
author | Zhizhou Zhang <zhizhouzhang@asrmicro.com> | 2018-11-21 11:01:43 +0800 |
---|---|---|
committer | Pengguang Zhu <pengguang.zhu@amlogic.com> | 2019-05-14 19:03:10 +0800 |
commit | 31f0ae9cecde555eaba749a4de2fe24eb944cc95 (patch) | |
tree | 61eb0d1cf110610f07abe067841dd5e857d44481 | |
parent | bd683566bbdf9eda7b030867aeeb255aa2d1b2dc (diff) | |
download | optee_linuxdriver-31f0ae9cecde555eaba749a4de2fe24eb944cc95.tar.gz |
tee: optee: avoid possible double list_del() [1/1]
PD#OTT-3799
Problem:
This bug occurs when:
- a new request arrives, one thread(let's call it A) is pending in
optee_supp_req() with req->busy is initial value false.
- tee-supplicant is killed, then optee_supp_release() is called, this
function calls list_del(&req->link), and set supp->ctx to NULL. And
it also wake up process A.
- process A continues, it firstly checks supp->ctx which is NULL,
then checks req->busy which is false, at last run list_del(&req->link).
This triggers double list_del() and results kernel panic.
Solution:
For solve this problem, we rename req->busy to req->in_queue, and
associate it with state of whether req is linked to supp->reqs. So we
can just only check req->in_queue to make decision calling list_del()
or not.
Verify:
Android P + S922X
Change-Id: I14a71d1bda933573c7216774954c08fa3161be6c
Signed-off-by: Zhizhou Zhang <zhizhouzhang@asrmicro.com>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
-rwxr-xr-x | optee/supp.c | 13 |
1 files changed, 7 insertions, 6 deletions
diff --git a/optee/supp.c b/optee/supp.c index cea1101..d4f4a34 100755 --- a/optee/supp.c +++ b/optee/supp.c @@ -19,7 +19,7 @@ struct optee_supp_req { struct list_head link; - bool busy; + bool in_queue; u32 func; u32 ret; size_t num_params; @@ -54,7 +54,6 @@ void optee_supp_release(struct optee_supp *supp) /* Abort all request retrieved by supplicant */ idr_for_each_entry(&supp->idr, req, id) { - req->busy = false; idr_remove(&supp->idr, id); req->ret = TEEC_ERROR_COMMUNICATION; complete(&req->c); @@ -63,6 +62,7 @@ void optee_supp_release(struct optee_supp *supp) /* Abort all queued requests */ list_for_each_entry_safe(req, req_tmp, &supp->reqs, link) { list_del(&req->link); + req->in_queue = false; req->ret = TEEC_ERROR_COMMUNICATION; complete(&req->c); } @@ -107,6 +107,7 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params, /* Insert the request in the request list */ mutex_lock(&supp->mutex); list_add_tail(&req->link, &supp->reqs); + req->in_queue = true; mutex_unlock(&supp->mutex); /* Tell an eventual waiter there's a new request */ @@ -134,9 +135,10 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params, * will serve all requests in a timely manner and * interrupting then wouldn't make sense. */ - interruptable = !req->busy; - if ((!req->busy) && (req->ret != TEEC_ERROR_COMMUNICATION)) + if (req->in_queue) { list_del(&req->link); + req->in_queue = false; + } } mutex_unlock(&supp->mutex); @@ -180,7 +182,7 @@ static struct optee_supp_req *supp_pop_entry(struct optee_supp *supp, return ERR_PTR(-ENOMEM); list_del(&req->link); - req->busy = true; + req->in_queue = false; return req; } @@ -322,7 +324,6 @@ static struct optee_supp_req *supp_pop_req(struct optee_supp *supp, if ((num_params - nm) != req->num_params) return ERR_PTR(-EINVAL); - req->busy = false; idr_remove(&supp->idr, id); supp->req_id = -1; *num_meta = nm; |