Skip to content

Commit c605ea4

Browse files
ChidvilasYQCfabiobaltieri
authored andcommitted
kernel: thread monitor - Assert reusing TCB in use
Add a check in thread monitor to assert if a TCB is reused while it is still present in the kernel thread list. This prevents undefined behavior due to TID reuse before proper cleanup. Signed-off-by: Chidvilas Yerramsetti <cyerrams@qti.qualcomm.com>
1 parent b6d1d04 commit c605ea4

2 files changed

Lines changed: 79 additions & 1 deletion

File tree

‎kernel/thread.c‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,30 @@ static inline void init_thread_usage(struct k_thread *thread)
873873
#endif /* CONFIG_SCHED_THREAD_USAGE */
874874
}
875875

876+
/* Assert the thread wasn't already linked into the monitor list. */
877+
static inline void assert_thread_not_reused(struct k_thread *new_thread)
878+
{
879+
#ifdef CONFIG_THREAD_MONITOR
880+
#ifdef CONFIG_ASSERT
881+
k_spinlock_key_t key = k_spin_lock(&z_thread_monitor_lock);
882+
bool reused = false;
883+
884+
/* Check if the thread is already in the list */
885+
for (struct k_thread *t = _kernel.threads; t; t = t->next_thread) {
886+
if (t == new_thread) {
887+
reused = true;
888+
break;
889+
}
890+
}
891+
892+
k_spin_unlock(&z_thread_monitor_lock, key);
893+
894+
__ASSERT(!reused, "thread %p is already in the running list", new_thread);
895+
#endif /* CONFIG_ASSERT */
896+
#endif /* CONFIG_THREAD_MONITOR */
897+
ARG_UNUSED(new_thread);
898+
}
899+
876900
/*
877901
* The provided stack_size value is presumed to be either the result of
878902
* K_THREAD_STACK_SIZEOF(stack), or the size value passed to the instance
@@ -888,6 +912,8 @@ char *z_setup_new_thread(struct k_thread *new_thread,
888912

889913
Z_ASSERT_VALID_PRIO(prio, entry);
890914

915+
assert_thread_not_reused(new_thread);
916+
891917
thread_abort_cleanup_check_reuse(new_thread);
892918
init_thread_obj_core(new_thread);
893919
init_thread_userspace(new_thread, stack, options);

‎tests/kernel/threads/thread_error_case/src/main.c‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@ static ZTEST_DMEM int case_type;
1414

1515
static K_THREAD_STACK_DEFINE(tstack, STACK_SIZE);
1616
static K_THREAD_STACK_DEFINE(test_stack, STACK_SIZE);
17+
static K_THREAD_STACK_DEFINE(reuse_stack1, STACK_SIZE);
18+
static K_THREAD_STACK_DEFINE(reuse_stack2, STACK_SIZE);
1719
static struct k_thread tdata;
1820
static struct k_thread test_tdata;
21+
static struct k_thread reuse_tdata;
1922

2023
/* identifiers for each negative test scenario */
2124
enum {
@@ -32,14 +35,21 @@ enum {
3235
THREAD_PRIORITY_GET_NULL,
3336
THREAD_WAKEUP_NULL,
3437
THREAD_CREATE_SUPERVISOR,
35-
THREAD_CREATE_ESSENTIAL
38+
THREAD_CREATE_ESSENTIAL,
39+
THREAD_REUSE_ACTIVE_THREAD_ID
3640
} neg_case;
3741

3842
static void test_thread(void *p1, void *p2, void *p3)
3943
{
4044
/* intentionally empty target thread */
4145
}
4246

47+
static void long_running_thread(void *p1, void *p2, void *p3)
48+
{
49+
/* Sleep for a long time to keep thread active */
50+
k_sleep(K_FOREVER);
51+
}
52+
4353
static void tThread_entry_negative(void *p1, void *p2, void *p3)
4454
{
4555
ARG_UNUSED(p2);
@@ -149,6 +159,16 @@ static void tThread_entry_negative(void *p1, void *p2, void *p3)
149159
K_PRIO_PREEMPT(THREAD_TEST_PRIORITY),
150160
K_USER | K_ESSENTIAL, K_NO_WAIT);
151161
break;
162+
case THREAD_REUSE_ACTIVE_THREAD_ID:
163+
ztest_set_fault_valid(true);
164+
/* Create first thread that will run for a while */
165+
k_thread_create(&reuse_tdata, reuse_stack1, STACK_SIZE, long_running_thread, NULL,
166+
NULL, NULL, K_PRIO_PREEMPT(THREAD_TEST_PRIORITY + 1), perm,
167+
K_NO_WAIT);
168+
/* Try to reuse the same thread structure while it's still active */
169+
k_thread_create(&reuse_tdata, reuse_stack2, STACK_SIZE, test_thread, NULL, NULL,
170+
NULL, K_PRIO_PREEMPT(THREAD_TEST_PRIORITY), perm, K_NO_WAIT);
171+
break;
152172
default:
153173
TC_PRINT("should not be here!\n");
154174
break;
@@ -382,6 +402,38 @@ ZTEST_USER(thread_error_case, test_thread_create_stack_size_overflow)
382402
create_negative_test_thread(THREAD_CREATE_STACK_SIZE_OVERFLOW);
383403
}
384404

405+
/**
406+
* @brief Verify that k_thread_create() asserts if a thread pointer is
407+
* reused while the thread pointer is in use.
408+
*
409+
* @ingroup kernel_thread_tests
410+
*
411+
* @details
412+
* A thread structure must not be reused for a new thread while the
413+
* thread it currently backs is still active, so this must be detected
414+
* instead of silently corrupting the running thread's state.
415+
*
416+
* This test runs in kernel mode, a worker thread arms the ztest
417+
* fatal-error hook and then makes the offending call. The call must
418+
* not return -- reaching the code after it fails the test -- so a pass
419+
* means the error really was caught.
420+
*
421+
* Test steps:
422+
* - Spawn a worker thread and arm the ztest fatal-error hook.
423+
* - Create a first thread that keeps running, then try to reuse its thread
424+
* structure to create a second thread from the same worker thread.
425+
* - Join the worker thread.
426+
*
427+
* Expected result:
428+
* - The call raises the expected fatal error and never returns.
429+
*
430+
* @see k_thread_create()
431+
*/
432+
ZTEST(thread_error_case, test_thread_reuse_active_thread_id)
433+
{
434+
create_negative_test_thread(THREAD_REUSE_ACTIVE_THREAD_ID);
435+
}
436+
385437
/**
386438
* @brief Verify that k_thread_suspend() rejects a NULL thread pointer.
387439
*

0 commit comments

Comments
 (0)