Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TA to TA communication results in segfault #28

Open
hkandiga opened this issue Feb 2, 2022 · 3 comments
Open

TA to TA communication results in segfault #28

hkandiga opened this issue Feb 2, 2022 · 3 comments

Comments

@hkandiga
Copy link

hkandiga commented Feb 2, 2022

from one TA to another TA results in segfault during TEE_InvokeTACommand

I know that the APIs for TEE_OpenSession, TEE_InvokeTACommand have been added since "TEE Internal Core API v1.2".

Just wanted to know if there any plan to support the TA to TA communication?

@TanelDettenborn
Copy link
Member

There was a bug related to TA2TA communication and TEE_TYPE_MEMREF_XXXX, but it did not end up to segfault. The fix has been committed into tee-engine-repository.

Would you be able to provide reproducing steps?

Unfortunately currently OpenTEE allows ta2ta communication when connection is established and therefore it is possible only within "TA_InvokeCommandEntryPoint"-function. It needs a bit investigating whether it is feasible to add other entrypoint-functions.

As a trivia then "4.9 Internal Client API"-chapter has been in since TEE Internal Core API v1.0 (I guess from the first spec).

A bug fix tee-engine
Open-TEE/tee-engine@e8f42a4

A basic smoke test for TA2TA communication
Open-TEE/TAs@0c497fd
Open-TEE/tests@646b329

@hkandiga
Copy link
Author

hkandiga commented Feb 7, 2022

@TanelDettenborn
Thanks for the fix.
The usecase was that one TA was opening session and invoking commands in its own OpenSession call.

With the latest code, I see the following observation.
msg gets dropped with "tee_manager: ../../emulator/manager/logic_thread.c:invoke_cmd:921 Invalid sender or senders status". Since the TA's status is proc_initialized.

Please consider the following patch which fixes the issue.

From a84de0298d7f8653a3c95ab5e5509d533c389a14 Mon Sep 17 00:00:00 2001
From: Harish Jenny K N <[email protected]>
Date: Mon, 7 Feb 2022 13:55:04 +0530
Subject: [PATCH] logic_thread: TA to TA communication in OpenSession of a TA

Allowed TA invoke command from another TA during opensession
and closesession.

Signed-off-by: Harish Jenny K N <[email protected]>
Change-Id: I7f6695980a0d41cb9dbba635c8f0138fe5df5e0b
---
 manager/logic_thread.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/manager/logic_thread.c b/manager/logic_thread.c
index 0b47b24..dd63c7f 100644
--- a/manager/logic_thread.c
+++ b/manager/logic_thread.c
@@ -918,8 +918,11 @@ static void invoke_cmd(struct manager_msg *man_msg)
 
 	/* Function is only valid for proc FDs */
 	if (man_msg->proc->p_type == proc_t_session || man_msg->proc->status != proc_active) {
-		OT_LOG(LOG_ERR, "Invalid sender or senders status");
-		goto discard_msg;
+		/* Allow TA invoke command from another TA during opensession */
+		if (!(man_msg->proc->p_type == proc_t_TA && man_msg->proc->status == proc_initialized)) {
+			OT_LOG(LOG_ERR, "Invalid sender or senders status");
+			goto discard_msg;
+		}
 	}
 
 	/* REsponse to invoke to command can be received only from TA */
@@ -1535,8 +1538,11 @@ static void close_session(struct manager_msg *man_msg)
 
 	/* Function is only valid for proc FDs */
 	if (man_msg->proc->p_type == proc_t_session || man_msg->proc->status != proc_active) {
-		OT_LOG(LOG_ERR, "Invalid sender or senders status");
-		goto ignore_msg;
+		/* Allow TA closesession during proc_initialized */
+		if (!(man_msg->proc->p_type == proc_t_TA && man_msg->proc->status == proc_initialized)) {
+			OT_LOG(LOG_ERR, "Invalid sender or senders status");
+			goto ignore_msg;
+		}
 	}
 
 	session = get_sesLink_by_ID(man_msg->proc, close_msg->msg_hdr.sess_id);
-- 
2.17.1

@TanelDettenborn
Copy link
Member

@hkandiga
It has been a while and therefore I need to sit down and think through changes, because what I can recall rational behind ta2ta communication limitation was a reliability. As a retrospect the previous design choice has not been optimal. It might limit too much usability.

Above patch might be enough, but I will think through and try to get the patch in as soon as possible!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants