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-supplicant: Support multiple ta load paths #306

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions config.mk
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ CFG_TEE_CLIENT_LOG_FILE ?= $(CFG_TEE_FS_PARENT_PATH)/teec.log

# CFG_TEE_CLIENT_LOAD_PATH
# The location of the client library file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is just wrong, it should be removed. The same error is present in CMakeLists.mk.

# Suport multiple ta load path
# CFG_TEE_CLIENT_LOAD_PATH ?= /lib:/vendor/lib
CFG_TEE_CLIENT_LOAD_PATH ?= /lib

# CFG_TEE_SUPP_PLUGINS
Expand Down
64 changes: 61 additions & 3 deletions tee-supplicant/src/tee_supplicant.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@
#define RPC_BUF_SIZE (sizeof(struct tee_iocl_supp_send_arg) + \
RPC_NUM_PARAMS * sizeof(struct tee_ioctl_param))

char **ta_prefix;
int num_ta_prefix;

union tee_rpc_invoke {
uint64_t buf[(RPC_BUF_SIZE - 1) / sizeof(uint64_t) + 1];
struct tee_iocl_supp_recv_arg recv;
Expand Down Expand Up @@ -701,6 +704,10 @@ int main(int argc, char *argv[])
int e = 0;
int long_index = 0;
int opt = 0;
char *test_ta_prefix_multipath = NULL;
char *ta_prefix_multipath = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the variables is a bit hard to figure out from their names IMO.

char *temp;
int i, len;

e = pthread_mutex_init(&arg.mutex, NULL);
if (e) {
Expand Down Expand Up @@ -762,30 +769,81 @@ int main(int argc, char *argv[])
exit(EXIT_FAILURE);
}

/* Support multiple ta load paths */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather have a helper function to parse the paths. There is duplication.

#ifdef TEEC_TEST_LOAD_PATH
if (TEEC_TEST_LOAD_PATH) {
num_ta_prefix++;
len = strlen(TEEC_TEST_LOAD_PATH);
for (i = 0; i < len; i++) {
if (TEEC_TEST_LOAD_PATH[i] == ':') {
num_ta_prefix++;
}
}
}
#endif
if (TEEC_LOAD_PATH) {
num_ta_prefix++;
len = strlen(TEEC_LOAD_PATH);
for (i = 0; i < len; i++) {
if (TEEC_LOAD_PATH[i] == ':') {
num_ta_prefix++;
}
}
}
ta_prefix = (char **)malloc(sizeof(char *) * num_ta_prefix);
if (!ta_prefix) {
EMSG("out of memory");
goto exit;
}

i = 0;
#ifdef TEEC_TEST_LOAD_PATH
test_ta_prefix_multipath = strdup(TEEC_TEST_LOAD_PATH);
if (!test_ta_prefix) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_ta_prefix_multipath

EMSG("out of memory");
goto exit;
}
while ((temp = strsep(&test_ta_prefix_multipath, ":")) != NULL) {
ta_prefix[i++] = temp;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ta_prefix[i] may be an empty string if multiple colons are found in a row.

#endif
ta_prefix_multipath = strdup(TEEC_LOAD_PATH);
if (!ta_prefix_multipath) {
EMSG("out of memory");
goto exit;
}
while ((temp = strsep(&ta_prefix_multipath, ":")) != NULL) {
ta_prefix[i++] = temp;
}

if (dev) {
arg.fd = open_dev(dev, &arg.gen_caps);
if (arg.fd < 0) {
EMSG("failed to open \"%s\"", argv[1]);
exit(EXIT_FAILURE);
goto exit;
}
} else {
arg.fd = get_dev_fd(&arg.gen_caps);
if (arg.fd < 0) {
EMSG("failed to find an OP-TEE supplicant device");
exit(EXIT_FAILURE);
goto exit;
}
}

if (plugin_load_all() != 0) {
EMSG("failed to load plugins");
exit(EXIT_FAILURE);
goto exit;
}

while (!arg.abort) {
if (!process_one_request(&arg))
arg.abort = true;
}

exit:
free(test_ta_prefix_multipath);
free(ta_prefix_multipath);
free(ta_prefix);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced this kind of cleanup is desirable immediately before exiting.

close(arg.fd);

return EXIT_FAILURE;
Expand Down
18 changes: 8 additions & 10 deletions tee-supplicant/src/teec_ta_load.c
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,13 @@ int TEECI_LoadSecureModule(const char* dev_path,
const TEEC_UUID *destination, void *ta,
size_t *ta_size)
{
#ifdef TEEC_TEST_LOAD_PATH
int res = 0;
int res = TA_BINARY_NOT_FOUND;

res = try_load_secure_module(TEEC_TEST_LOAD_PATH,
dev_path, destination, ta, ta_size);
if (res != TA_BINARY_NOT_FOUND)
return res;
#endif

return try_load_secure_module(TEEC_LOAD_PATH,
dev_path, destination, ta, ta_size);
for (int i = 0; i < num_ta_prefix; i++) {
res = try_load_secure_module(ta_prefix[i],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, the documentation for try_load_secure_module() lacks the description of its first two arguments.

dev_path, destination, ta, ta_size);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix indentation

		res = try_load_secure_module(ta_prefix[i], dev_path,
					     destination, ta, ta_size);

if (res == TA_BINARY_FOUND)
break;
}
return res;
}
4 changes: 4 additions & 0 deletions tee-supplicant/src/teec_ta_load.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
#define TA_BINARY_FOUND 0
#define TA_BINARY_NOT_FOUND -1

/* Support multiple TA load paths */
extern char **ta_prefix;
extern int num_ta_prefix;

/**
* Based on the uuid this function will try to find a TA-binary on the
* filesystem and return it back to the caller in the parameter ta.
Expand Down