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

SingleInstance, MultiSession TA is loaded multiple times #6801

Open
KhOlegDc opened this issue Apr 22, 2024 · 9 comments · May be fixed by #6836
Open

SingleInstance, MultiSession TA is loaded multiple times #6801

KhOlegDc opened this issue Apr 22, 2024 · 9 comments · May be fixed by #6836

Comments

@KhOlegDc
Copy link

Hello,

I have two CAs, connecting to the same TA. I want that only one TA instance will be in memory, serving requests from both CAs. I understand that requests will be served sequentially, that's OK and even desired. The idea is to share TA's RAM (heap objects) when serving requests from different CAs.

To achieve this, I configured TA:

#define TA_FLAGS      (TA_FLAG_SINGLE_INSTANCE | TA_FLAG_MULTI_SESSION)

and initialized each of CAs:

    TEEC_Result res = TEEC_InitializeContext(NULL, &teec_context);
    if (res != TEEC_SUCCESS) {
        return ERROR;
    }

    uint32_t resOrigin = 0;
    res = TEEC_OpenSession(&teec_context, &teec_session, &uuid, TEEC_LOGIN_PUBLIC, NULL, NULL,
                           &resOrigin);
    if (res != TEEC_SUCCESS) {
        TEEC_FinalizeContext(&teec_context);
        return ERROR;
    }
    return OK;

I run both CAs, making sure that sessions from both overlap in time. However, in the log I see that a separate TA instance is loaded for each CA session:

D/LD:  ldelf:176 ELF (fbaecc2c-xxxx-xxxx-xxxx-xxxxxxxxxxxx) at 0x4005a000  <=== first instance
E/TA:  TA_CreateEntryPoint:40 TA_CreateEntryPoint called
E/TA:  TA_CreateEntryPoint:48 Property SingleInstance is set to true
E/TA:  TA_CreateEntryPoint:57 Property MultiSession is set to true
E/TA:  TA_OpenSessionEntryPoint:84 TA_OpenSessionEntryPoint called
E/TA:  TA_OpenSessionEntryPoint:95 Session opened, TA version: 1.1.0
D/LD:  ldelf:176 ELF (fbaecc2c-xxxx-xxxx-xxxx-xxxxxxxxxxxx) at 0x4003d000   <=== second one
E/TA:  TA_CreateEntryPoint:40 TA_CreateEntryPoint called
E/TA:  TA_CreateEntryPoint:48 Property SingleInstance is set to true
E/TA:  TA_CreateEntryPoint:57 Property MultiSession is set to true
E/TA:  TA_OpenSessionEntryPoint:84 TA_OpenSessionEntryPoint called
E/TA:  TA_OpenSessionEntryPoint:95 Session opened, TA version: 1.1.0
E/TA:  TA_InvokeCommandEntryPoint:115 TA_InvokeCommandEntryPoint called: METHOD1
E/TA:  TA_InvokeCommandEntryPoint:115 TA_InvokeCommandEntryPoint called: METHOD2
E/TA:  TA_InvokeCommandEntryPoint:115 TA_InvokeCommandEntryPoint called: METHOD1
E/TA:  TA_InvokeCommandEntryPoint:115 TA_InvokeCommandEntryPoint called: METHOD1
*D/TC:? 1 do_notif:36 sleep thread 1 0x978a1348
D/TC:? 0 do_notif:36 wake  thread 1 0x978a1348
E/TA:  TA_InvokeCommandEntryPoint:115 TA_InvokeCommandEntryPoint called: METHOD2
D/TC:? 0 tee_ta_close_session:529 csess 0x978b88d0 id 3
D/TC:? 0 tee_ta_close_session:548 Destroy session
E/TA:  TA_CloseSessionEntryPoint:106 TA_CloseSessionEntryPoint called
D/TC:? 1 do_notif:36 sleep thread 1 0x978a1348
*E/TA:  TA_CloseSessionEntryPoint:109 Session closed
E/TA:  TA_DestroyEntryPoint:70 TA_DestroyEntryPoint called
D/TC:? 0 do_notif:36 wake  thread 1 0x978a1348
D/TC:? 0 destroy_context:326 Destroy TA ctx (0x978b8360)
D/TC:? 0 do_notif:36 sleep thread 0 0x978a1108
*D/TC:? 1 do_notif:36 wake  thread 0 0x978a1108
E/TA:  TA_InvokeCommandEntryPoint:115 TA_InvokeCommandEntryPoint called: METHOD1
D/TC:? 0 tee_ta_close_session:529 csess 0x978b8980 id 2
D/TC:? 0 tee_ta_close_session:548 Destroy session
E/TA:  TA_CloseSessionEntryPoint:106 TA_CloseSessionEntryPoint called
E/TA:  TA_CloseSessionEntryPoint:109 Session closed
E/TA:  TA_DestroyEntryPoint:70 TA_DestroyEntryPoint called
D/TC:? 0 destroy_context:326 Destroy TA ctx (0x978b8870)

OP-TEE v3.22, running on qemu_v8

Could you suggest something to have only one TA instance in memory?

@etienne-lms
Copy link
Contributor

Hi @KhOlegDc, I don't understand.
With #define TA_FLAGS (TA_FLAG_SINGLE_INSTANCE | TA_FLAG_MULTI_SESSION) there should be a single instance of the TA (unless TA crash at 1st creation, id does not seems the case in your test). Make sure to TA loaded is the one you rebuilt (located in a filesystem, in optee secure storage or built-in/early TA?).

@KhOlegDc
Copy link
Author

There is only one TA available and it's my one. To make sure that TA_FLAGS are correctly passed to the TA during build, I added check for these values:

TEE_Result TA_CreateEntryPoint(void)
{
    EMSG("TA_CreateEntryPoint called");

    bool value = false;
    TEE_Result res = TEE_GetPropertyAsBool(TEE_PROPSET_CURRENT_TA, "gpd.ta.singleInstance", &value);

    if (res != TEE_SUCCESS) {
        EMSG("Error reading TA property, res: %u", res);
    } else {
        EMSG("Property SingleInstance is set to %s", value ? "true" : "false");
    }

    value = false;
    res = TEE_GetPropertyAsBool(TEE_PROPSET_CURRENT_TA, "gpd.ta.multiSession", &value);

    if (res != TEE_SUCCESS) {
        EMSG("Error reading TA property, res: %u", res);
    } else {
        EMSG("Property MultiSession is set to %s", value ? "true" : "false");
    }
    return TEE_SUCCESS;
}

you can see the output in the log:

E/TA:  TA_CreateEntryPoint:48 Property SingleInstance is set to true
E/TA:  TA_CreateEntryPoint:57 Property MultiSession is set to true

This setup runs in QEMU, I start the VM, connect to it via SSH and upload the TA to /lib/optee_armtz. After that I upload and run CAs.

@KhOlegDc
Copy link
Author

KhOlegDc commented May 6, 2024

The same behaviour is observed with OP-TEE v4.2.0 - calls from two CAs result in two TA instances loaded in memory.
@etienne-lms, do you need any help with reproducing the issue?

@etienne-lms
Copy link
Contributor

I'll try to reproduce on qemu_virt or qemu_armv8a.

@etienne-lms
Copy link
Contributor

Hi @KhOlegDc,
I've tested on vexpress-qemu_armv8a platform, hacking optee_test/ta/crypt/ to make it single-instance/multi_session and adding a TEE_Wait() call in the TA, then running xtest regression_1004 & twice. I found no issues, the TA is load once, function tee_ta_context_find() successfully found the existing already loaded TA instance.

@KhOlegDc
Copy link
Author

Hi @etienne-lms,
Thank you for your investigations, I will try to repeat your experiment with optee_test/ta/crypt.
But could, you, please, tell, how did you check that no second instance was loaded? The function tee_ta_context_find() returns the first context with the given UUID, if there are several. Could you, maybe share your tee.log. I am interested, how may times TA_CreateEntryPoint is called. My expectation is that it must be called only once.

@etienne-lms
Copy link
Contributor

I applied this change on optee_test:

diff --git a/ta/crypt/include/user_ta_header_defines.h b/ta/crypt/include/user_ta_header_defines.h
index ac83ae05f..ea0ce9a54 100644
--- a/ta/crypt/include/user_ta_header_defines.h
+++ b/ta/crypt/include/user_ta_header_defines.h
@@ -11,7 +11,7 @@
 
 #define TA_UUID TA_CRYPT_UUID
 
-#define TA_FLAGS               (TA_FLAG_USER_MODE | TA_FLAG_EXEC_DDR)
+#define TA_FLAGS               (TA_FLAG_SINGLE_INSTANCE | TA_FLAG_MULTI_SESSION)
 #define TA_STACK_SIZE          (32 * 1024)
 #define TA_DATA_SIZE           (32 * 1024)
 
diff --git a/ta/crypt/ta_entry.c b/ta/crypt/ta_entry.c
index b997b0c7e..8bd6ce2e3 100644
--- a/ta/crypt/ta_entry.c
+++ b/ta/crypt/ta_entry.c
@@ -70,6 +70,8 @@ TEE_Result TA_InvokeCommandEntryPoint(void *pSessionContext,
 
        (void)pSessionContext;
 
+       TEE_Wait(1000);
+
        switch (nCommandID) {
        case TA_CRYPT_CMD_SHA224:
                use_fptr = !use_fptr;

and run from Linux console { xtest regression_1004 & } ; sleep 3; xtest regression_1004

The OP-TEE debug level logs show 2 sessions & 1 TA context:

D/TC:? 0 tee_ta_init_pseudo_ta_session:297 Lookup pseudo TA cb3e5ba0-adf1-11e0-998b-0002a5d5c51b
D/TC:? 0 ldelf_load_ldelf:110 ldelf load address 0x40007000
D/LD:  ldelf:142 Loading TS cb3e5ba0-adf1-11e0-998b-0002a5d5c51b
D/TC:? 0 ldelf_syscall_open_bin:163 Lookup user TA ELF cb3e5ba0-adf1-11e0-998b-0002a5d5c51b (early TA)
D/TC:? 0 ldelf_syscall_open_bin:167 res=0xffff0008
D/TC:? 0 ldelf_syscall_open_bin:163 Lookup user TA ELF cb3e5ba0-adf1-11e0-998b-0002a5d5c51b (Secure Storage TA)
D/TC:? 0 ldelf_syscall_open_bin:167 res=0xffff0008
D/TC:? 0 ldelf_syscall_open_bin:163 Lookup user TA ELF cb3e5ba0-adf1-11e0-998b-0002a5d5c51b (REE)
D/TC:? 0 ldelf_syscall_open_bin:167 res=0
D/LD:  ldelf:176 ELF (cb3e5ba0-adf1-11e0-998b-0002a5d5c51b) at 0x4004a000
D/TC:? 1 tee_ta_init_session_with_context:559 Re-open TA cb3e5ba0-adf1-11e0-998b-0002a5d5c51b
D/TC:? 1 do_notif:34 sleep thread 1 0x85800400
D/TC:? 0 do_notif:34 wake  thread 1 0x85800400
D/TC:? 1 do_notif:34 sleep thread 1 0x85800400
D/TC:? 0 do_notif:34 wake  thread 1 0x85800400
D/TC:? 0 do_notif:34 sleep thread 0 0x85800400
D/TC:? 1 do_notif:34 wake  thread 0 0x85800400
D/TC:? 1 do_notif:34 sleep thread 1 0x85800400
D/TC:? 0 do_notif:34 wake  thread 1 0x85800400
D/TC:? 0 do_notif:34 sleep thread 0 0x85800400
D/TC:? 1 do_notif:34 wake  thread 0 0x85800400
D/TC:? 1 tee_ta_close_session:461 csess 0x85817820 id 2
D/TC:? 1 tee_ta_close_session:480 Destroy session
D/TC:? 1 do_notif:34 sleep thread 1 0x85800400
D/TC:? 0 do_notif:34 wake  thread 1 0x85800400
D/TC:? 0 tee_ta_close_session:461 csess 0x85815a00 id 3
D/TC:? 0 tee_ta_close_session:480 Destroy session
D/TC:? 0 destroy_context:318 Destroy TA ctx (0x858177c0)

@KhOlegDc
Copy link
Author

KhOlegDc commented May 14, 2024

In my case I also could achieve that only one instance gets loaded. When I run two copies this way:

my_app &
my_app &

I observe that two instances are loaded as shown in the log in my first message. When I run the applications this way:

my_app &
sleep 1
my_app &

I see that only one instance is loaded:

D/LD:  ldelf:176 ELF (fbaecc2c-xxxx-xxxx-xxxx-xxxxxxxxxxxx) at 0x4001e000
E/TA:  TA_CreateEntryPoint:73 TA_CreateEntryPoint called
E/TA:  print_properties:44 Property SingleInstance is set to true
E/TA:  print_properties:53 Property MultiSession is set to true
E/TA:  print_properties:62 Property InstanceKeepAlive is set to false
E/TA:  TA_OpenSessionEntryPoint:109 TA_OpenSessionEntryPoint called
E/TA:  TA_OpenSessionEntryPoint:117 Session opened, TA version: 1.1.0
==> D/TC:? 0 tee_ta_init_session_with_context:624 Re-open TA fbaecc2c-xxxx-xxxx-xxxx-xxxxxxxxxxxx <==
E/TA:  TA_OpenSessionEntryPoint:109 TA_OpenSessionEntryPoint called
E/TA:  TA_OpenSessionEntryPoint:117 Session opened, TA version: 1.1.0

My hypothesis is that there is a race. The context is only then gets visible to tee_ta_context_find, when it's completely initialized. When two contexts are being created at the same time, TEE doesn't detect this situation and loads TA twice.
If you remove sleep 3 from your example, you should be able to reproduce the issue.

In my case, sleep 1 is an acceptable workaround, but strictly speaking two copies of TA while SINGLE_INSTANCE flag is set seems to be a bug.

etienne-lms added a commit to etienne-lms/optee_os that referenced this issue May 14, 2024
Fix race condition on single instance TA creation where several
instances of a single instance TA could be created if invoked closed
enough that they are created after tee_ta_init_session() calls
tee_ta_init_session_with_context().

Closes: OP-TEE#6801
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
etienne-lms added a commit to etienne-lms/optee_os that referenced this issue May 14, 2024
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: OP-TEE#6801
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
@etienne-lms
Copy link
Contributor

Indeed! nice catch, thanks. I should have tried without this sleep command :(
I've created #6836 to address this issue.

etienne-lms added a commit to etienne-lms/optee_os that referenced this issue May 14, 2024
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: OP-TEE#6801
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
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

Successfully merging a pull request may close this issue.

2 participants