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

TEE Internal API bug report #28

Open
littlec84 opened this issue Jun 14, 2016 · 0 comments
Open

TEE Internal API bug report #28

littlec84 opened this issue Jun 14, 2016 · 0 comments
Assignees

Comments

@littlec84
Copy link

While trying to use the Internal API communication (OpenTASession, InvokeTACommand,...) I incurred in several issues.

file launcher/ta_internal_thread.c, function ta2ta_com_msg_op_to_params, line 234:

sizeof(sizeof(params[i].value)));

it should be

sizeof(params[i].value))


same file, function get_shm_from_manager_and_map_region, lines 290-307

_if (open_shm->msg_hdr.msg_name != COM_MSG_NAME_OPEN_SHM_REGION) {

    if (!get_vals_from_err_msg(response_msg, &ret, NULL)) {
        OT_LOG(LOG_ERR, "Received unknown message");
        ret = TEE_ERROR_GENERIC;
    }

    /* Received error message */
    goto err;
}

if (open_shm->msg_hdr.shareable_fd_count != 1) {
    OT_LOG(LOG_ERR, "wrong number of file descriptors");
    goto err;
}

if (open_shm->return_code != TEE_SUCCESS)
    goto err;_

These lines manage some error cases but when such errors happen, the function will return TEE_SUCCESS all the same (except the case of "received unknown message").
The calling function map_and_cpy_parameters will assume that everything is OK and proceed to memcpy into a not mapped/not existing shared buffer => Segmentation Fault
To solve such issue I added some more lines like

ret = TEE_ERROR_GENERIC;


same file, function copy_com_msg_op_to_param, lines 760-767

 _if (TEE_PARAM_TYPE_GET(param_types, i) == TEEC_MEMREF_TEMP_OUTPUT ||
            TEE_PARAM_TYPE_GET(param_types, i) ==    TEEC_MEMREF_PARTIAL_OUTPUT ||
            TEE_PARAM_TYPE_GET(param_types, i) == TEE_PARAM_TYPE_MEMREF_OUTPUT) {
            isOutput = true;
        } else {
            isOutput = false;
        }_

I don't understand the meaning of these lines. The isOutput variable is passed to open_shared_mem and, when it is true, the shared buffer is opened as read-only. Why is it so?
This make impossible to use output buffer when using internal API, while it still work for client API, if using WHOLE buffers.
I temporarly solved by commenting these lines and setting isOutput to false.


file launcher/ta_io_thread.c, function receive_from_manager, line 238-245:

_if (msg_type == COM_TYPE_RESPONSE) {
    response_msg = new_ta_task->msg;
    free(new_ta_task);

    while (header->shareable_fd_count > 0) {
        header->shareable_fd_count--;
        close(header->shareable_fd[header->shareable_fd_count]);
    }_

Why, if the received message is a response, the shareable fds are closed? Indeed this provoke errors whenever a TA send an open shmem message to the manager. I temporarly solved by commenting the while loop.


Finally a feature request: is it possible to add an implementation of Property Access Functions to the Internal APIs?

Thanks

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