Skip to content
Permalink
Browse files

core: do not use virtual addresses as session identifier

Session context virtual address is returned to the REE in
entry_open_session(); it is then used back in entry_close_session() and
entry_invoke_command(). Sharing virtual addresses with the REE leads to
virtual memory addresses disclosure that could be leveraged to defeat
ASLR (if/when implemented) and/or mount an attack.

Similarly, syscall_open_ta_session() returns a session ID directly
derived from the session virtual address to the caller TA.

This commit introduces a 32-bit identifier field in struct tee_ta_session.
The ID is generated when the session is created, starting from the id of
the last session in the queue, and counting up until a number that is not
used in the session queue is found.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [2.1]
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>
  • Loading branch information...
jforissier committed Feb 4, 2019
1 parent 781c8f0 commit 99164a05ff515a077ff0f3e1550838d24623665b
Showing with 66 additions and 14 deletions.
  1. +2 −2 core/arch/arm/tee/entry_std.c
  2. +4 −0 core/include/kernel/tee_ta_manager.h
  3. +56 −8 core/kernel/tee_ta_manager.c
  4. +4 −4 core/tee/tee_svc.c
@@ -330,7 +330,7 @@ static void entry_open_session(struct thread_smc_args *smc_args,

out:
if (s)
arg->session = (vaddr_t)s;
arg->session = s->id;
else
arg->session = 0;
arg->ret = res;
@@ -352,7 +352,7 @@ static void entry_close_session(struct thread_smc_args *smc_args,
plat_prng_add_jitter_entropy(CRYPTO_RNG_SRC_JITTER_SESSION,
&session_pnum);

s = (struct tee_ta_session *)(vaddr_t)arg->session;
s = tee_ta_find_session(arg->session, &tee_open_sessions);
res = tee_ta_close_session(s, &tee_open_sessions, NSAPP_IDENTITY);
out:
arg->ret = res;
@@ -90,6 +90,7 @@ struct tee_ta_ctx {
struct tee_ta_session {
TAILQ_ENTRY(tee_ta_session) link;
TAILQ_ENTRY(tee_ta_session) link_tsd;
uint32_t id; /* Session handle (0 is invalid) */
struct tee_ta_ctx *ctx; /* TA context */
TEE_Identity clnt_id; /* Identify of client */
bool cancel; /* True if TAF is cancelled */
@@ -149,6 +150,9 @@ struct tee_ta_session *tee_ta_pop_current_session(void);

struct tee_ta_session *tee_ta_get_calling_session(void);

struct tee_ta_session *tee_ta_find_session(uint32_t id,
struct tee_ta_session_head *open_sessions);

struct tee_ta_session *tee_ta_get_session(uint32_t id, bool exclusive,
struct tee_ta_session_head *open_sessions);

@@ -173,16 +173,34 @@ void tee_ta_put_session(struct tee_ta_session *s)
mutex_unlock(&tee_ta_mutex);
}

static struct tee_ta_session *find_session(uint32_t id,
static struct tee_ta_session *tee_ta_find_session_nolock(uint32_t id,
struct tee_ta_session_head *open_sessions)
{
struct tee_ta_session *s;
struct tee_ta_session *s = NULL;
struct tee_ta_session *found = NULL;

TAILQ_FOREACH(s, open_sessions, link) {
if ((vaddr_t)s == id)
return s;
if (s->id == id) {
found = s;
break;
}
}
return NULL;

return found;
}

struct tee_ta_session *tee_ta_find_session(uint32_t id,
struct tee_ta_session_head *open_sessions)
{
struct tee_ta_session *s = NULL;

mutex_lock(&tee_ta_mutex);

s = tee_ta_find_session_nolock(id, open_sessions);

mutex_unlock(&tee_ta_mutex);

return s;
}

struct tee_ta_session *tee_ta_get_session(uint32_t id, bool exclusive,
@@ -193,7 +211,7 @@ struct tee_ta_session *tee_ta_get_session(uint32_t id, bool exclusive,
mutex_lock(&tee_ta_mutex);

while (true) {
s = find_session(id, open_sessions);
s = tee_ta_find_session_nolock(id, open_sessions);
if (!s)
break;
if (s->unlink) {
@@ -377,12 +395,12 @@ TEE_Result tee_ta_close_session(struct tee_ta_session *csess,
struct tee_ta_ctx *ctx;
bool keep_alive;

DMSG("tee_ta_close_session(0x%" PRIxVA ")", (vaddr_t)csess);
DMSG("csess 0x%" PRIxVA " id %u", (vaddr_t)csess, csess->id);

if (!csess)
return TEE_ERROR_ITEM_NOT_FOUND;

sess = tee_ta_get_session((vaddr_t)csess, true, open_sessions);
sess = tee_ta_get_session(csess->id, true, open_sessions);

if (!sess) {
EMSG("session 0x%" PRIxVA " to be removed is not found",
@@ -463,6 +481,31 @@ static TEE_Result tee_ta_init_session_with_context(struct tee_ta_ctx *ctx,
return TEE_SUCCESS;
}

static uint32_t new_session_id(struct tee_ta_session_head *open_sessions)
{
struct tee_ta_session *last = NULL;
uint32_t saved = 0;
uint32_t id = 1;

last = TAILQ_LAST(open_sessions, tee_ta_session_head);
if (last) {
/* This value is less likely to be already used */
id = last->id + 1;
if (!id)
id++; /* 0 is not valid */
}

saved = id;
do {
if (!tee_ta_find_session_nolock(id, open_sessions))
return id;
id++;
if (!id)
id++;
} while (id != saved);

return 0;
}

static TEE_Result tee_ta_init_session(TEE_ErrorOrigin *err,
struct tee_ta_session_head *open_sessions,
@@ -492,6 +535,11 @@ static TEE_Result tee_ta_init_session(TEE_ErrorOrigin *err,


mutex_lock(&tee_ta_mutex);
s->id = new_session_id(open_sessions);
if (!s->id) {
res = TEE_ERROR_OVERFLOW;
goto out;
}
TAILQ_INSERT_TAIL(open_sessions, s, link);

/* Look for already loaded TA */
@@ -789,7 +789,7 @@ TEE_Result syscall_open_ta_session(const TEE_UUID *dest,
function_exit:
mobj_free(mobj_param);
if (res == TEE_SUCCESS)
tee_svc_copy_kaddr_to_uref(ta_sess, s);
tee_svc_copy_to_user(ta_sess, &s->id, sizeof(s->id));
tee_svc_copy_to_user(ret_orig, &ret_o, sizeof(ret_o));

out_free_only:
@@ -804,13 +804,14 @@ TEE_Result syscall_close_ta_session(unsigned long ta_sess)
TEE_Result res;
struct tee_ta_session *sess;
TEE_Identity clnt_id;
struct tee_ta_session *s = tee_svc_uref_to_kaddr(ta_sess);
struct tee_ta_session *s = NULL;
struct user_ta_ctx *utc;

res = tee_ta_get_current_session(&sess);
if (res != TEE_SUCCESS)
return res;
utc = to_user_ta_ctx(sess->ctx);
s = tee_ta_find_session(ta_sess, &utc->open_sessions);

clnt_id.login = TEE_LOGIN_TRUSTED_APP;
memcpy(&clnt_id.uuid, &sess->ctx->uuid, sizeof(TEE_UUID));
@@ -838,8 +839,7 @@ TEE_Result syscall_invoke_ta_command(unsigned long ta_sess,
return res;
utc = to_user_ta_ctx(sess->ctx);

called_sess = tee_ta_get_session(
(vaddr_t)tee_svc_uref_to_kaddr(ta_sess), true,
called_sess = tee_ta_get_session((uint32_t)ta_sess, true,
&utc->open_sessions);
if (!called_sess)
return TEE_ERROR_BAD_PARAMETERS;

0 comments on commit 99164a0

Please sign in to comment.
You can’t perform that action at this time.