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

[patch]TEEC_MEMREF_TEMP_OUTPUT type buffer #22

Open
gaurav4dev opened this issue Nov 9, 2016 · 1 comment
Open

[patch]TEEC_MEMREF_TEMP_OUTPUT type buffer #22

gaurav4dev opened this issue Nov 9, 2016 · 1 comment

Comments

@gaurav4dev
Copy link

gaurav4dev commented Nov 9, 2016

Hi All,

TEEC_MEMREF_TEMP_OUTPUT type buffer seems can't be written from TA side.
When I modify "conn_test_app" example in CA/TA side to use TEEC_MEMREF_TEMP_OUTPUT type of buffer, it crashes during open_session in TA as temp buffers with TEEC_MEMREF_TEMP_OUTPUT flags are not writeable,

//modify CA "conn_test_app" example
--- a/conn_test_app/conn_test_app.c
+++ b/conn_test_app/conn_test_app.c
@@ -96,7 +96,7 @@ static void fill_operation_params(TEEC_Operation *operation,
TEEC_SharedMemory *alloc_inout_mem)
{
operation->paramTypes = TEEC_PARAM_TYPES(TEEC_VALUE_INOUT, TEEC_MEMREF_WHOLE,

  •                                            TEEC_MEMREF_WHOLE, TEEC_MEMREF_TEMP_INOUT);
    
  •                                            TEEC_MEMREF_WHOLE, TEEC_MEMREF_TEMP_OUTPUT);
    
      /* Value parameter */
      operation->params[0].value.a  = IN_VALUE_A;
    

@@ -123,7 +123,7 @@ static int check_operation_params(TEEC_Operation *operation)
uint32_t i;

    if (operation->paramTypes != TEEC_PARAM_TYPES(TEEC_VALUE_INOUT, TEEC_MEMREF_WHOLE,
  •                                                 TEEC_MEMREF_WHOLE, TEEC_MEMREF_TEMP_INOUT)) {
    
  •                                                 TEEC_MEMREF_WHOLE, TEEC_MEMREF_TEMP_OUTPUT)) {
              PRIn("Not expected paramTypes");
              return 1;
      }
    

//modify TA "conn_test_app" example
--- a/ta_conn_test_app/ta_conn_test_app.c
+++ b/ta_conn_test_app/ta_conn_test_app.c
@@ -77,7 +78,7 @@ static TEE_Result check_recv_params(uint32_t paramTypes,

    if (TEE_PARAM_TYPE_GET(paramTypes, 1) != TEE_PARAM_TYPE_MEMREF_INOUT ||
        TEE_PARAM_TYPE_GET(paramTypes, 2) != TEE_PARAM_TYPE_MEMREF_INOUT ||
  •       TEE_PARAM_TYPE_GET(paramTypes, 3) != TEE_PARAM_TYPE_MEMREF_INOUT) {
    
  •       TEE_PARAM_TYPE_GET(paramTypes, 3) != TEE_PARAM_TYPE_MEMREF_OUTPUT) {
              OT_LOG(LOG_ERR, "Expected buffer inout type as index 1,2,3 parameter");
              return TEE_ERROR_BAD_PARAMETERS;
      }
    

//Issue
Crash happens in ta_conn_test_app.c static void reverse_buffer(
buffer[i] = buffer[j];

stack//
0 reverse_buffer ta_conn_test_app.c 59 0xb77af813
1 fill_response_params ta_conn_test_app.c 135 0xb77afb23
2 handle_params ta_conn_test_app.c 147 0xb77afb6d
3 TA_OpenSessionEntryPoint ta_conn_test_app.c 189 0xb77afcb7
4 open_session ta_internal_thread.c 831 0xb77bc0dd

I found TEEC_MEMREF_TEMP_OUTPUT is used to decide is "mmap" is done to TA for READ_ONLY, it seems confusing from TAs point of view so I changed it as below and it works. I am new to GP-spec and Open-TEE, so want your feedback for this issue if it's the correct way to use TEEC_MEMREF_TEMP_OUTPUT flag and if below is the correct way to fix it .

//My solution
--- a/launcher/ta_internal_thread.c
+++ b/launcher/ta_internal_thread.c
@@ -760,6 +760,7 @@ static int copy_com_msg_op_to_param(struct com_msg_operation *operation, TEE_Par

            } else {

+#if 0
/* determine if this is a readonly memory area */
if (TEE_PARAM_TYPE_GET(param_types, i) == TEEC_MEMREF_TEMP_OUTPUT ||
TEE_PARAM_TYPE_GET(param_types, i) == TEEC_MEMREF_PARTIAL_OUTPUT ||
@@ -768,11 +769,20 @@ static int copy_com_msg_op_to_param(struct com_msg_operation *operation, TEE_Par
} else {
isOutput = false;
}

+#else

  •        /* determine if this is a readonly memory area */
    
  •        if (TEE_PARAM_TYPE_GET(param_types, i) == TEEC_MEMREF_TEMP_INPUT ||
    
  •            TEE_PARAM_TYPE_GET(param_types, i) == TEEC_MEMREF_PARTIAL_INPUT ||
    
  •            TEE_PARAM_TYPE_GET(param_types, i) == TEE_PARAM_TYPE_MEMREF_INPUT) {
    
  •            isOutput = true; //actually in this case this should be renamed as isInput
    
  •        } else {
    
  •            isOutput = false;
    
  •        }
    

+#endif


(plain text description can be found in this attchment : https://github.com/Open-TEE/project/files/580115/temp_output_bug.txt)
Regards,
Gaurav

@gaurav4dev
Copy link
Author

temp_output_bug.txt

This is the plain text file of above description.

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

1 participant