libutee: Fix the init_done status while closing session

[zhupg]

Mark init_done as false if uninit_instance is called. This is necessary to avoid running a TA uninitialized. Also, need to call __utee_gprof_fini() and __utee_call_elf_fini_fn() next time opening TA session.

[jforissier]

Hi @zhupg, thanks for the patch. Could you please explain the use case? The current code pretty much assumes that initialization happens only once: the executable is supposed to be unmapped once uninit_instance() (including TA_DestroyEntryPoint()) has been called. Edit: now I see this is related to #7542 .

[jforissier]

We have ta_mutex to protect the TA state from race conditions and somehow I feel it should address this problem too.

[jenswi-linaro]

This should not be addressed in the TA. OP-TEE Core should prevent this from happening.
When TA_CreateEntryPoint() is called, it's expected that global variables have their initial values. An eventual residue from an earlier might cause problems.

[zhupg]

@jforissier @jenswi-linaro thanks for your reply.

It's better to fix it in OP-TEE core.
To prevent it in core, I think all of this code needs to be completed in one atomic operation. If using tee_ta_mutex for it, there will be two issues.

1. different TA's close session can not run concurrent.
2. It may hold the global tee_ta_mutex for a long time because the duration of enter_close_session() is uncertain.
   

   optee_os/core/kernel/tee_ta_manager.c

   Lines 489 to 520 in 3edbed4

   	ctx = ts_to_ta_ctx(ts_ctx);
   	if (ctx->panicked) {
   	destroy_session(sess, open_sessions);
   	} else {
   	tee_ta_set_busy(ctx);
   	set_invoke_timeout(sess, TEE_TIMEOUT_INFINITE);
   	ts_ctx->ops->enter_close_session(&sess->ts_sess);
   	destroy_session(sess, open_sessions);
   	tee_ta_clear_busy(ctx);
   	}
   	mutex_lock(&tee_ta_mutex);
   	if (ctx->ref_count <= 0)
   	panic();
   	ctx->ref_count--;
   	if (ctx->flags & TA_FLAG_SINGLE_INSTANCE)
   	keep_alive = ctx->flags & TA_FLAG_INSTANCE_KEEP_ALIVE;
   	if (keep_alive)
   	keep_crashed = ctx->flags & TA_FLAG_INSTANCE_KEEP_CRASHED;
   	if (!ctx->ref_count &&
   	((ctx->panicked && !keep_crashed) || !keep_alive)) {
   	if (!ctx->is_releasing) {
   	TAILQ_REMOVE(&tee_ctxes, ctx, link);
   	ctx->is_releasing = true;
   	}
   	mutex_unlock(&tee_ta_mutex);
   	destroy_context(ctx);
   	} else
   	mutex_unlock(&tee_ta_mutex);

Do you have any suggestions or can you provide a patch? thanks.

[jenswi-linaro]

We'll need another state variable in struct tee_ta_ctx to tell tee_ta_init_session_with_context() that the context is busy closing a session.

Later, when the session is closed and we know that the context should be kept, we can clear that variable and let tee_ta_init_session_with_context() try again. We'll need to call condvar_broadcast(&tee_ta_init_cv) regardless to wake up eventual threads waiting in tee_ta_init_session_with_context().

This is a rough sketch, but it should do the trick.

[github-actions]

This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.