Revert "core: fine grained tee_ta_mutex locking"

Commit 99f969d ("core: fine grained tee_ta_mutex locking") fixes a
deadlock that can occur if a TA is loaded while not enough page tables
are available in pgt_cache to map the context. But it also splits up a
big critical section and there's obviously a few hidden dependencies
towards tee_ta_mutex causing stability issues with the pager. Running
'while xtest 1013; do true; done' in AArch64 with at least three
threads running in parallel will ultimately fail.

Therefore, revert the fine grained locking commit until the race
conditions are sorted out.

Reported-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>

core/arch/arm/include/kernel/pseudo_ta.h

@@ -58,7 +58,8 @@ static inline struct pseudo_ta_ctx *to_pseudo_ta_ctx(struct tee_ta_ctx *ctx)
 	return container_of(ctx, struct pseudo_ta_ctx, ctx);
 }
 
-TEE_Result pseudo_ta_get_ctx(const TEE_UUID *uuid, struct tee_ta_ctx **ctx);
+TEE_Result tee_ta_init_pseudo_ta_session(const TEE_UUID *uuid,
+			struct tee_ta_session *s);
 
 #endif /* KERNEL_PSEUDO_TA_H */
 

core/arch/arm/include/kernel/user_ta.h

@@ -60,11 +60,13 @@ static inline struct user_ta_ctx *to_user_ta_ctx(struct tee_ta_ctx *ctx)
 struct user_ta_store_ops;
 
 #ifdef CFG_WITH_USER_TA
-TEE_Result user_ta_get_ctx(const TEE_UUID *uuid, struct tee_ta_ctx **ctx);
+TEE_Result tee_ta_init_user_ta_session(const TEE_UUID *uuid,
+			struct tee_ta_session *s);
 TEE_Result tee_ta_register_ta_store(struct user_ta_store_ops *ops);
 #else
-static inline TEE_Result user_ta_get_ctx(const TEE_UUID *uuid __unused,
-					 struct tee_ta_ctx **ctx __unused)
+static inline TEE_Result tee_ta_init_user_ta_session(
+			const TEE_UUID *uuid __unused,
+			struct tee_ta_session *s __unused)
 {
 	return TEE_ERROR_ITEM_NOT_FOUND;
 }

core/arch/arm/kernel/pseudo_ta.c

@@ -279,8 +279,12 @@ static TEE_Result verify_pseudo_tas_conformance(void)
 
 service_init(verify_pseudo_tas_conformance);
 
-/* Initializes and returns a TA context based on the UUID of a pseudo TA. */
-TEE_Result pseudo_ta_get_ctx(const TEE_UUID *uuid, struct tee_ta_ctx **ctx_ret)
+/*-----------------------------------------------------------------------------
+ * Initialises a session based on the UUID or ptr to the ta
+ * Returns ptr to the session (ta_session) and a TEE_Result
+ *---------------------------------------------------------------------------*/
+TEE_Result tee_ta_init_pseudo_ta_session(const TEE_UUID *uuid,
+			struct tee_ta_session *s)
 {
 	struct pseudo_ta_ctx *stc = NULL;
 	struct tee_ta_ctx *ctx;
@@ -305,13 +309,13 @@ TEE_Result pseudo_ta_get_ctx(const TEE_UUID *uuid, struct tee_ta_ctx **ctx_ret)
 	ctx = &stc->ctx;
 
 	ctx->ref_count = 1;
+	s->ctx = ctx;
 	ctx->flags = ta->flags;
 	stc->pseudo_ta = ta;
 	ctx->uuid = ta->uuid;
 	ctx->ops = &pseudo_ta_ops;
-	tee_ta_register_ctx(ctx);
+	TAILQ_INSERT_TAIL(&tee_ctxes, ctx, link);
 
-	*ctx_ret = ctx;
 	DMSG("%s : %pUl", stc->pseudo_ta->name, (void *)&ctx->uuid);
 
 	return TEE_SUCCESS;

core/arch/arm/kernel/user_ta.c

@@ -320,7 +320,7 @@ static TEE_Result ta_load(const TEE_UUID *uuid,
 	utc->entry_func = ta_head->entry.ptr64;
 	utc->ctx.ref_count = 1;
 	condvar_init(&utc->ctx.busy_cv);
-	tee_ta_register_ctx(&utc->ctx);
+	TAILQ_INSERT_TAIL(&tee_ctxes, &utc->ctx, link);
 	*ta_ctx = &utc->ctx;
 
 	tee_mmu_set_ctx(NULL);
@@ -626,19 +626,20 @@ TEE_Result tee_ta_register_ta_store(struct user_ta_store_ops *ops)
 	return TEE_SUCCESS;
 }
 
-TEE_Result user_ta_get_ctx(const TEE_UUID *uuid, struct tee_ta_ctx **ctx)
+TEE_Result tee_ta_init_user_ta_session(const TEE_UUID *uuid,
+			struct tee_ta_session *s)
 {
 	const struct user_ta_store_ops *store;
 	TEE_Result res;
 
 	SLIST_FOREACH(store, &uta_store_list, link) {
 		DMSG("Lookup user TA %pUl (%s)", (void *)uuid,
 		     store->description);
-		res = ta_load(uuid, store, ctx);
+		res = ta_load(uuid, store, &s->ctx);
 		if (res == TEE_ERROR_ITEM_NOT_FOUND)
 			continue;
 		if (res == TEE_SUCCESS)
-			(*ctx)->ops = &user_ta_ops;
+			s->ctx->ops = &user_ta_ops;
 		else
 			DMSG("res=0x%x", res);
 		return res;

core/include/kernel/tee_ta_manager.h

@@ -129,7 +129,10 @@ struct tee_ta_session {
 #endif
 };
 
-void tee_ta_register_ctx(struct tee_ta_ctx *ctx);
+/* Registered contexts */
+extern struct tee_ta_ctx_head tee_ctxes;
+
+extern struct mutex tee_ta_mutex;
 
 TEE_Result tee_ta_open_session(TEE_ErrorOrigin *err,
 			       struct tee_ta_session **sess,

core/kernel/tee_ta_manager.c

@@ -55,8 +55,8 @@
 #include <util.h>
 
 /* This mutex protects the critical section in tee_ta_init_session */
-static struct mutex tee_ta_mutex = MUTEX_INITIALIZER;
-static struct tee_ta_ctx_head tee_ctxes = TAILQ_HEAD_INITIALIZER(tee_ctxes);
+struct mutex tee_ta_mutex = MUTEX_INITIALIZER;
+struct tee_ta_ctx_head tee_ctxes = TAILQ_HEAD_INITIALIZER(tee_ctxes);
 
 #ifndef CFG_CONCURRENT_SINGLE_INSTANCE_TA
 static struct condvar tee_ta_cv = CONDVAR_INITIALIZER;
@@ -460,11 +460,9 @@ TEE_Result tee_ta_close_session(struct tee_ta_session *csess,
 	return TEE_SUCCESS;
 }
 
-static TEE_Result get_ctx(struct tee_ta_ctx *ctx)
+static TEE_Result tee_ta_init_session_with_context(struct tee_ta_ctx *ctx,
+			struct tee_ta_session *s)
 {
-	if (!ctx)
-		return TEE_ERROR_ITEM_NOT_FOUND;
-
 	/*
 	 * If TA isn't single instance it should be loaded as new
 	 * instance instead of doing anything with this instance.
@@ -484,15 +482,10 @@ static TEE_Result get_ctx(struct tee_ta_ctx *ctx)
 	DMSG("Re-open TA %pUl", (void *)&ctx->uuid);
 
 	ctx->ref_count++;
+	s->ctx = ctx;
 	return TEE_SUCCESS;
 }
 
-void tee_ta_register_ctx(struct tee_ta_ctx *ctx)
-{
-	mutex_lock(&tee_ta_mutex);
-	TAILQ_INSERT_TAIL(&tee_ctxes, ctx, link);
-	mutex_unlock(&tee_ta_mutex);
-}
 
 static TEE_Result tee_ta_init_session(TEE_ErrorOrigin *err,
 				struct tee_ta_session_head *open_sessions,
@@ -507,43 +500,48 @@ static TEE_Result tee_ta_init_session(TEE_ErrorOrigin *err,
 	if (!s)
 		return TEE_ERROR_OUT_OF_MEMORY;
 
+	s->cancel_mask = true;
+	condvar_init(&s->refc_cv);
+	condvar_init(&s->lock_cv);
+	s->lock_thread = THREAD_ID_INVALID;
+	s->ref_count = 1;
+
+
+	/*
+	 * We take the global TA mutex here and hold it while doing
+	 * RPC to load the TA. This big critical section should be broken
+	 * down into smaller pieces.
+	 */
+
 
-	/* Look for already loaded TA */
 	mutex_lock(&tee_ta_mutex);
+	TAILQ_INSERT_TAIL(open_sessions, s, link);
+
+	/* Look for already loaded TA */
 	ctx = tee_ta_context_find(uuid);
-	res = get_ctx(ctx);
-	mutex_unlock(&tee_ta_mutex);
-	if (res != TEE_ERROR_ITEM_NOT_FOUND)
-		goto out;
+	if (ctx) {
+		res = tee_ta_init_session_with_context(ctx, s);
+		if (res == TEE_SUCCESS || res != TEE_ERROR_ITEM_NOT_FOUND)
+			goto out;
+	}
 
 	/* Look for static TA */
-	res = pseudo_ta_get_ctx(uuid, &ctx);
-	if (res != TEE_ERROR_ITEM_NOT_FOUND)
+	res = tee_ta_init_pseudo_ta_session(uuid, s);
+	if (res == TEE_SUCCESS || res != TEE_ERROR_ITEM_NOT_FOUND)
 		goto out;
 
 	/* Look for user TA */
-	res = user_ta_get_ctx(uuid, &ctx);
+	res = tee_ta_init_user_ta_session(uuid, s);
+
 out:
-	if (res) {
+	if (res == TEE_SUCCESS) {
+		*sess = s;
+	} else {
+		TAILQ_REMOVE(open_sessions, s, link);
 		free(s);
-		return res;
 	}
-
-	/*
-	 * We have a context, let's finish initialization of the session
-	 */
-	s->cancel_mask = true;
-	condvar_init(&s->refc_cv);
-	condvar_init(&s->lock_cv);
-	s->lock_thread = THREAD_ID_INVALID;
-	s->ref_count = 1;
-	s->ctx = ctx;
-	mutex_lock(&tee_ta_mutex);
-	TAILQ_INSERT_TAIL(open_sessions, s, link);
 	mutex_unlock(&tee_ta_mutex);
-	*sess = s;
-
-	return TEE_SUCCESS;
+	return res;
 }
 
 TEE_Result tee_ta_open_session(TEE_ErrorOrigin *err,