From 31f0ae9cecde555eaba749a4de2fe24eb944cc95 Mon Sep 17 00:00:00 2001 From: Zhizhou Zhang Date: Wed, 21 Nov 2018 11:01:43 +0800 Subject: [PATCH] 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 Signed-off-by: Jens Wiklander --- optee/supp.c | 13 +++++++------ 1 file 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; -- 2.20.1