diff --git a/tee/include/tee/tee_svc.h b/tee/include/tee/tee_svc.h index 324ad3f4..923af5bd 100644 --- a/tee/include/tee/tee_svc.h +++ b/tee/include/tee/tee_svc.h @@ -88,6 +88,11 @@ TEE_Result syscall_invoke_ta_command(unsigned long sess, TEE_Result syscall_check_access_rights(unsigned long flags, const void *buf, size_t len); +struct user_ta_ctx; + +TEE_Result tee_mmu_check_access_rights(struct user_ta_ctx *utc, uint32_t flags, + uaddr_t uaddr, size_t len); + TEE_Result tee_svc_copy_from_user(void *kaddr, const void *uaddr, size_t len); TEE_Result tee_svc_copy_to_user(void *uaddr, const void *kaddr, size_t len); diff --git a/tee/tee/tee_svc.c b/tee/tee/tee_svc.c index 9da66bbe..4910bbac 100644 --- a/tee/tee/tee_svc.c +++ b/tee/tee/tee_svc.c @@ -926,41 +926,74 @@ vaddr_t tee_svc_uref_base = 0; // (uaddr_t)buf, len); //} // +/* + * tee_mmu_check_access_rights - validate a TA-supplied pointer range. + * + * In this ARMv8-M port TAs are statically linked into the secure image and run + * privileged in a single flat address space: there is no per-TA MMU/MPU context + * or region table (utc->mmu is not populated), so the OP-TEE-style ownership and + * secure/non-secure attribution this signature was designed for have no backing + * state. Every live caller requests TEE_MEMORY_ACCESS_ANY_OWNER, i.e. "do not + * enforce per-TA ownership, just accessibility", so this validates the property + * that is actually meaningful here: the range is a real, non-wrapping range. This + * mirrors the wrap/overflow guard cmse_check_address_range() performs on the + * non-secure boundary and rejects the dangerous primitive of an attacker- or + * bug-supplied length that would drive an out-of-bounds copy. + */ +TEE_Result tee_mmu_check_access_rights(struct user_ta_ctx *utc, uint32_t flags, + uaddr_t uaddr, size_t len) +{ + (void)utc; /* unused: no per-TA region data here; callers pass NULL */ + (void)flags; /* ANY_OWNER: ownership is not (and can not be) enforced */ + + if (!len) + return TEE_SUCCESS; /* zero-length range is trivially OK */ + if (!uaddr) + return TEE_ERROR_ACCESS_DENIED; + /* + * Reject a range that wraps the address space. len is non-zero here, so + * the last byte is at uaddr + len - 1; if that is below uaddr the range + * overflowed. Matches cmse_check_address_range()'s end-of-range test and + * still accepts a range whose last byte sits at the top of memory. + */ + if (uaddr + len - 1 < uaddr) + return TEE_ERROR_ACCESS_DENIED; + + return TEE_SUCCESS; +} + TEE_Result tee_svc_copy_from_user(void *kaddr, const void *uaddr, size_t len) { -// TEE_Result res; -// struct tee_ta_session *s; -// -// res = tee_ta_get_current_session(&s); -// if (res != TEE_SUCCESS) -// return res; -// -// res = tee_mmu_check_access_rights(to_user_ta_ctx(s->ctx), -// TEE_MEMORY_ACCESS_READ | -// TEE_MEMORY_ACCESS_ANY_OWNER, -// (uaddr_t)uaddr, len); -// if (res != TEE_SUCCESS) -// return res; -// + TEE_Result res; + + /* + * No session lookup here: tee_mmu_check_access_rights() ignores the utc + * in this port, and tee_ta_get_current_session() returns TEE_SUCCESS with + * a NULL current_session when no TA session is active, so dereferencing + * it (to_user_ta_ctx(s->ctx)) would fault instead of failing gracefully. + */ + res = tee_mmu_check_access_rights(NULL, + TEE_MEMORY_ACCESS_READ | + TEE_MEMORY_ACCESS_ANY_OWNER, + (uaddr_t)uaddr, len); + if (res != TEE_SUCCESS) + return res; + memcpy(kaddr, uaddr, len); return TEE_SUCCESS; } TEE_Result tee_svc_copy_to_user(void *uaddr, const void *kaddr, size_t len) { -// TEE_Result res; -// struct tee_ta_session *s; -// -// res = tee_ta_get_current_session(&s); -// if (res != TEE_SUCCESS) -// return res; -// -// res = tee_mmu_check_access_rights(to_user_ta_ctx(s->ctx), -// TEE_MEMORY_ACCESS_WRITE | -// TEE_MEMORY_ACCESS_ANY_OWNER, -// (uaddr_t)uaddr, len); -// if (res != TEE_SUCCESS) -// return res; + TEE_Result res; + + /* See tee_svc_copy_from_user() on why no session is looked up here. */ + res = tee_mmu_check_access_rights(NULL, + TEE_MEMORY_ACCESS_WRITE | + TEE_MEMORY_ACCESS_ANY_OWNER, + (uaddr_t)uaddr, len); + if (res != TEE_SUCCESS) + return res; memcpy(uaddr, kaddr, len); return TEE_SUCCESS; diff --git a/tee/tee/tee_svc_cryp.c b/tee/tee/tee_svc_cryp.c index be24999f..d3eb0877 100644 --- a/tee/tee/tee_svc_cryp.c +++ b/tee/tee/tee_svc_cryp.c @@ -1337,15 +1337,19 @@ static TEE_Result copy_in_attrs(struct user_ta_ctx *utc, const struct utee_attribute *usr_attrs, uint32_t attr_count, TEE_Attribute *attrs) { -// TEE_Result res; + TEE_Result res; uint32_t n; - (void) utc; -// res = tee_mmu_check_access_rights(utc, -// TEE_MEMORY_ACCESS_READ | TEE_MEMORY_ACCESS_ANY_OWNER, -// (uaddr_t)usr_attrs, -// attr_count * sizeof(struct utee_attribute)); -// if (res != TEE_SUCCESS) -// return res; + size_t attrs_size; + + /* Guard the range length against a 32-bit multiply wrap. */ + if (MUL_OVERFLOW(attr_count, sizeof(struct utee_attribute), &attrs_size)) + return TEE_ERROR_OVERFLOW; + + res = tee_mmu_check_access_rights(utc, + TEE_MEMORY_ACCESS_READ | TEE_MEMORY_ACCESS_ANY_OWNER, + (uaddr_t)usr_attrs, attrs_size); + if (res != TEE_SUCCESS) + return res; for (n = 0; n < attr_count; n++) { attrs[n].attributeID = usr_attrs[n].attribute_id; @@ -1356,11 +1360,11 @@ static TEE_Result copy_in_attrs(struct user_ta_ctx *utc, uintptr_t buf = usr_attrs[n].a; size_t len = usr_attrs[n].b; -// res = tee_mmu_check_access_rights(utc, -// TEE_MEMORY_ACCESS_READ | -// TEE_MEMORY_ACCESS_ANY_OWNER, buf, len); -// if (res != TEE_SUCCESS) -// return res; + res = tee_mmu_check_access_rights(utc, + TEE_MEMORY_ACCESS_READ | + TEE_MEMORY_ACCESS_ANY_OWNER, buf, len); + if (res != TEE_SUCCESS) + return res; attrs[n].content.ref.buffer = (void *)buf; attrs[n].content.ref.length = len; } @@ -2231,12 +2235,12 @@ TEE_Result utee_hash_update(unsigned long state, const void *chunk, if (res != TEE_SUCCESS) return res; -// res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx), -// TEE_MEMORY_ACCESS_READ | -// TEE_MEMORY_ACCESS_ANY_OWNER, -// (uaddr_t)chunk, chunk_size); -// if (res != TEE_SUCCESS) -// return res; + res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx), + TEE_MEMORY_ACCESS_READ | + TEE_MEMORY_ACCESS_ANY_OWNER, + (uaddr_t)chunk, chunk_size); + if (res != TEE_SUCCESS) + return res; res = tee_svc_cryp_get_state(sess, tee_svc_uref_to_vaddr(state), &cs); if (res != TEE_SUCCESS) @@ -2277,24 +2281,24 @@ TEE_Result utee_hash_final(unsigned long state, const void *chunk, if (res != TEE_SUCCESS) return res; -// res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx), -// TEE_MEMORY_ACCESS_READ | -// TEE_MEMORY_ACCESS_ANY_OWNER, -// (uaddr_t)chunk, chunk_size); -// if (res != TEE_SUCCESS) -// return res; + res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx), + TEE_MEMORY_ACCESS_READ | + TEE_MEMORY_ACCESS_ANY_OWNER, + (uaddr_t)chunk, chunk_size); + if (res != TEE_SUCCESS) + return res; res = tee_svc_copy_from_user(&hlen, hash_len, sizeof(hlen)); if (res != TEE_SUCCESS) return res; -// res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx), -// TEE_MEMORY_ACCESS_READ | -// TEE_MEMORY_ACCESS_WRITE | -// TEE_MEMORY_ACCESS_ANY_OWNER, -// (uaddr_t)hash, hlen); -// if (res != TEE_SUCCESS) -// return res; + res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx), + TEE_MEMORY_ACCESS_READ | + TEE_MEMORY_ACCESS_WRITE | + TEE_MEMORY_ACCESS_ANY_OWNER, + (uaddr_t)hash, hlen); + if (res != TEE_SUCCESS) + return res; res = tee_svc_cryp_get_state(sess, tee_svc_uref_to_vaddr(state), &cs); if (res != TEE_SUCCESS) @@ -2373,13 +2377,13 @@ TEE_Result utee_cipher_init(unsigned long state, const void *iv, if (res != TEE_SUCCESS) return res; -// res = tee_mmu_check_access_rights(utc, -// TEE_MEMORY_ACCESS_READ | -// TEE_MEMORY_ACCESS_ANY_OWNER, -// (uaddr_t) iv, iv_len); -// if (res != TEE_SUCCESS) -// return res; -// + res = tee_mmu_check_access_rights(utc, + TEE_MEMORY_ACCESS_READ | + TEE_MEMORY_ACCESS_ANY_OWNER, + (uaddr_t) iv, iv_len); + if (res != TEE_SUCCESS) + return res; + res = tee_obj_get(utc, cs->key1, &o); if (res != TEE_SUCCESS) return res; @@ -2427,12 +2431,12 @@ static TEE_Result tee_svc_cipher_update_helper(unsigned long state, if (res != TEE_SUCCESS) return res; -// res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx), -// TEE_MEMORY_ACCESS_READ | -// TEE_MEMORY_ACCESS_ANY_OWNER, -// (uaddr_t)src, src_len); -// if (res != TEE_SUCCESS) -// return res; + res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx), + TEE_MEMORY_ACCESS_READ | + TEE_MEMORY_ACCESS_ANY_OWNER, + (uaddr_t)src, src_len); + if (res != TEE_SUCCESS) + return res; if (!dst_len) { dlen = 0; @@ -2441,13 +2445,13 @@ static TEE_Result tee_svc_cipher_update_helper(unsigned long state, if (res != TEE_SUCCESS) return res; -// res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx), -// TEE_MEMORY_ACCESS_READ | -// TEE_MEMORY_ACCESS_WRITE | -// TEE_MEMORY_ACCESS_ANY_OWNER, -// (uaddr_t)dst, dlen); -// if (res != TEE_SUCCESS) -// return res; + res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx), + TEE_MEMORY_ACCESS_READ | + TEE_MEMORY_ACCESS_WRITE | + TEE_MEMORY_ACCESS_ANY_OWNER, + (uaddr_t)dst, dlen); + if (res != TEE_SUCCESS) + return res; } if (dlen < src_len) {