Skip to content

Commit 7d41b40

Browse files
bmolodanclaude
authored andcommitted
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 = <secure address>, 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) <noreply@anthropic.com>
1 parent d71301d commit 7d41b40

1 file changed

Lines changed: 16 additions & 5 deletions

File tree

tee/kernel/entry_std.c

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,15 +93,20 @@ TEEC_Result tee_ioctl_close_session(/*ctx,*/ struct tee_ioctl_buf_data *buf_data
9393
* spuriously reject valid buffers. The 'write' argument is kept for a possible
9494
* future tightening if a Non-Secure MPU is ever set up.
9595
*
96-
* Returns p on success, NULL on failure. A zero-length range is trivially OK
97-
* (nothing is dereferenced).
96+
* Returns p on success, NULL on failure.
97+
*
98+
* A zero-length range dereferences nothing, but the pointer still reaches the
99+
* TA, and a TA that ignores the size would dereference it. A NULL pointer is
100+
* accepted (there is nothing to attribute); a non-NULL one is attribution-
101+
* checked a byte at a time so a *secure* pointer cannot ride through on
102+
* size == 0.
98103
*/
99104
static void *nsec_check(void *p, size_t len, int write)
100105
{
101106
(void)write;
102107

103108
if (len == 0)
104-
return p;
109+
return p ? cmse_check_address_range(p, 1, CMSE_NONSECURE) : p;
105110

106111
return cmse_check_address_range(p, len, CMSE_NONSECURE);
107112
}
@@ -283,7 +288,13 @@ static TEE_Result copy_in_params(const struct optee_msg_param *params,
283288
{
284289
// TEE_Result res;
285290
size_t n;
286-
uint8_t pt[TEE_NUM_PARAMS];
291+
/*
292+
* num_params is Non-Secure controlled and may be < TEE_NUM_PARAMS, but all
293+
* four entries are read below when building ta_param->types. Initialise to
294+
* TEE_PARAM_TYPE_NONE (0) so the unwritten slots cannot expose stale secure
295+
* stack contents to the TA as parameter types.
296+
*/
297+
uint8_t pt[TEE_NUM_PARAMS] = { 0 };
287298

288299
if (num_params > TEE_NUM_PARAMS)
289300
return TEE_ERROR_BAD_PARAMETERS;
@@ -577,7 +588,7 @@ TEEC_Result tee_ioctl_invoke(/*ctx,*/ struct tee_ioctl_buf_data *buf_data)
577588
goto out;
578589
}
579590

580-
res = tee_ta_invoke_command(&err, sess, arg->func, &param);
591+
res = tee_ta_invoke_command(&err, sess, func, &param);
581592

582593
copy_out_param(&param, num_params, arg->params, saved_attr);
583594

0 commit comments

Comments
 (0)