core: fix race condition on single instance TA loading #6836
Conversation
core/kernel/user_ta.c
Outdated
| @@ -486,6 +486,22 @@ TEE_Result tee_ta_init_user_ta_session(const TEE_UUID *uuid, | |||
| #endif | |||
|
|
|||
| mutex_lock(&tee_ta_mutex); | |||
|
|
|||
| /* | |||
| * Before updating the context list check again if a context | |||
There was a problem hiding this comment.
This is a problem applicable to PTAs as well.
There was a problem hiding this comment.
right, i missed that.
core/kernel/user_ta.c
Outdated
| */ | ||
| res = tee_ta_init_session_with_context(s, uuid); | ||
| if (res != TEE_ERROR_ITEM_NOT_FOUND) { | ||
| vm_info_final(&utc->uctx); |
There was a problem hiding this comment.
I'd prefer checking if the context is available before allocating the utc. That means holding the mutex during the call to calloc(), but that should be OK.
Replace field is_initializing from struct user_mode_ctx and struct stmm_ctx with new field is_initialing in generic struct tee_ta_ctx so that it can be use in generic context loading functions. Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Export function tee_ta_init_session_with_context() so that it can be called from TA/PTA/StMM context creation functions to check if the target context already exists. Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Fix race condition on creation of a context for single instance TAs, PTAs or StMM application. Such race condition could occur and lead to duplicated contexts if connected close enough that they are created after tee_ta_init_session() calls tee_ta_init_session_with_context() and before the context are added in the centralized context list. To prevent holding tee_ta_mutex during the load of the StMM application, the context is added in the generic context list at context allocation, relying on tee_ta_ctx:is_initializing to state when the context creation is fully completed. By the way, fix inline comment in pseudo_ta.c that refer to TA loading whereas it should refer to a pseudo TA context creation. Closes: OP-TEE#6801 Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
|
I've update the P-R to covert PTAs and StMM contexts creation. |
Fix typos. Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
|
|
| */ | ||
| if ((ctx->flags & TA_FLAG_SINGLE_INSTANCE) == 0) | ||
| return TEE_ERROR_ITEM_NOT_FOUND; | ||
| if (is_user_ta_ctx(&ctx->ts_ctx)) { |
There was a problem hiding this comment.
The flags TA_FLAG_SINGLE_INSTANCE and TA_FLAG_MULTI_SESSION apply to all TAs.
See for instance PTA_MANDATORY_FLAGS
| * Before updating the context list check again if a context | ||
| * already exists in the list for a single instance TA. | ||
| */ | ||
| res = tee_ta_init_session_with_context(s, uuid); |
There was a problem hiding this comment.
I don't like calling tee_ta_init_session_with_context() twice. How about splitting tee_ta_init_user_ta_session() (and stmm_init_session()) into a two-step operation? The first function, for instance tee_ta_init_user_ta_session() does everything up to the first mutex_unlock(), but with the mutex held by the calling function. The second function, tee_ta_complete_user_ta_session() is called after tee_ta_init_user_ta_session() has returned OK without the mutex held, instead it takes the mutex internally as needed.
Fix race condition on single instance TA creation where several instances of a single instance TA could be created if invoked close enough that they are both created after tee_ta_init_session() calls tee_ta_init_session_with_context().
Closes: #6801