From 85013f5f0c30f0d42d582ba4f4a0495b078b6f71 Mon Sep 17 00:00:00 2001 From: Bohdan Date: Tue, 25 Aug 2026 17:36:17 +0300 Subject: [PATCH] fix(tee): harden entry_std parameter handling Three independent defects on the Non-Secure entry path, found while investigating the HOTP key-size overflow. None is HOTP-specific, so they are split out from that fix. 1. pt[] is used uninitialised. copy_in_params() declares uint8_t pt[TEE_NUM_PARAMS] and assigns only pt[0..num_params-1], but all four entries are read when building ta_param->types. num_params is Non-Secure controlled, so a caller sending num_params < 4 hands the TA stale secure-stack bytes as parameter types; ta_param->u[] is zeroed, so a TA can observe e.g. MEMREF_INOUT with {buffer = NULL, size = 0}. Not reachable from the stock client, which always sends 4, but it is the hostile-caller model 27e314c addressed. Initialise to TEE_PARAM_TYPE_NONE (0). 2. Zero-length validation bypass in nsec_check(). The function returned its pointer unvalidated when len == 0, so a memref of {buffer = , size = 0} reached a TA with a secure pointer intact. The kernel does not dereference it, but the TA might, if it does not consult the size. Attribution-check non-NULL pointers one byte at a time; NULL and legitimate zero-length Non-Secure memrefs still pass. 3. Dead func snapshot. tee_ioctl_invoke() snapshots func = arg->func then passes arg->func, re-reading Non-Secure memory after validation. Harmless today, but it defeats the single-fetch discipline of 27e314c and leaves an unused variable. Pass the snapshot. Verified under QEMU (mps2-an505, cortex-m33): the NS->S attribution negative test still rejects all five crafted secure-pointer cases with ACCESS_DENIED, and the AES example still completes ("Clear text and decoded text match"), which is the guard against 2 over-tightening nsec_check. numaker_pfm_m2351 (armv8-m.base) builds clean. Co-Authored-By: Claude Opus 5 (1M context) --- tee/kernel/entry_std.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tee/kernel/entry_std.c b/tee/kernel/entry_std.c index 64250c2..642438e 100644 --- a/tee/kernel/entry_std.c +++ b/tee/kernel/entry_std.c @@ -93,15 +93,20 @@ TEEC_Result tee_ioctl_close_session(/*ctx,*/ struct tee_ioctl_buf_data *buf_data * spuriously reject valid buffers. The 'write' argument is kept for a possible * future tightening if a Non-Secure MPU is ever set up. * - * Returns p on success, NULL on failure. A zero-length range is trivially OK - * (nothing is dereferenced). + * Returns p on success, NULL on failure. + * + * A zero-length range dereferences nothing, but the pointer still reaches the + * TA, and a TA that ignores the size would dereference it. A NULL pointer is + * accepted (there is nothing to attribute); a non-NULL one is attribution- + * checked a byte at a time so a *secure* pointer cannot ride through on + * size == 0. */ static void *nsec_check(void *p, size_t len, int write) { (void)write; if (len == 0) - return p; + return p ? cmse_check_address_range(p, 1, CMSE_NONSECURE) : p; return cmse_check_address_range(p, len, CMSE_NONSECURE); } @@ -283,7 +288,13 @@ static TEE_Result copy_in_params(const struct optee_msg_param *params, { // TEE_Result res; size_t n; - uint8_t pt[TEE_NUM_PARAMS]; + /* + * num_params is Non-Secure controlled and may be < TEE_NUM_PARAMS, but all + * four entries are read below when building ta_param->types. Initialise to + * TEE_PARAM_TYPE_NONE (0) so the unwritten slots cannot expose stale secure + * stack contents to the TA as parameter types. + */ + uint8_t pt[TEE_NUM_PARAMS] = { 0 }; if (num_params > TEE_NUM_PARAMS) return TEE_ERROR_BAD_PARAMETERS; @@ -577,7 +588,7 @@ TEEC_Result tee_ioctl_invoke(/*ctx,*/ struct tee_ioctl_buf_data *buf_data) goto out; } - res = tee_ta_invoke_command(&err, sess, arg->func, ¶m); + res = tee_ta_invoke_command(&err, sess, func, ¶m); copy_out_param(¶m, num_params, arg->params, saved_attr);