From 3ef282be6c5d71514ca555e1a99eb3db2dc8eb6f Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Fri, 29 Mar 2024 07:37:35 -0400 Subject: [PATCH] tests/kernel/threads: Augment abort_from_isr test to detect FMW There's a easily-tripped-upon free memory write condition in the arch layers where they will write to a cached _current pointer after that thread has been aborted. Detect this by clobbering the thread data after the return from k_thread_abort() and validating that it's still clear after the ISR returns. Note that the clobbering of the thread struct requires the removal of a k_thread_join() that (obviously) requires that it see a DEAD flag set. Joining aborted threads isn't actually legal, but does still work as long as app code doesn't reuse the memory. Basically we can't test both cases here, and we have join coverage elsewhere already. Signed-off-by: Andy Ross --- .../src/test_threads_cancel_abort.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/kernel/threads/thread_apis/src/test_threads_cancel_abort.c b/tests/kernel/threads/thread_apis/src/test_threads_cancel_abort.c index aad8842a11e..b99347a404f 100644 --- a/tests/kernel/threads/thread_apis/src/test_threads_cancel_abort.c +++ b/tests/kernel/threads/thread_apis/src/test_threads_cancel_abort.c @@ -166,6 +166,11 @@ static void offload_func(const void *param) k_thread_abort(t); + /* Thread memory is unused now, validate that we can clobber it. */ + if (!IS_ENABLED(CONFIG_ARCH_POSIX)) { + memset(t, 0, sizeof(*t)); + } + /* k_thread_abort() in an isr shouldn't affect the ISR's execution */ isr_finished = true; } @@ -201,6 +206,19 @@ ZTEST(threads_lifecycle, test_abort_from_isr) k_thread_join(&tdata, K_FOREVER); zassert_true(isr_finished, "ISR did not complete"); + /* Thread struct was cleared after the abort, make sure it is + * still clear (i.e. that the arch layer didn't write to it + * during interrupt exit). Doesn't work on posix, which needs + * the thread struct for its swap code. + */ + uint8_t *p = (uint8_t *)&tdata; + + if (!IS_ENABLED(CONFIG_ARCH_POSIX)) { + for (int i = 0; i < sizeof(tdata); i++) { + zassert_true(p[i] == 0, "Free memory write to aborted thread"); + } + } + /* Notice: Recover back the offload_sem: This is use for releasing * offload_sem which might be held when thread aborts itself in ISR * context, it will cause irq_offload cannot be used again. @@ -248,6 +266,5 @@ ZTEST(threads_lifecycle, test_abort_from_isr_not_self) /* Simulate taking an interrupt which kills spwan thread */ irq_offload(offload_func, (void *)tid); - k_thread_join(&tdata, K_FOREVER); zassert_true(isr_finished, "ISR did not complete"); }