From c7bb423f3ee805eb5cbc71f75bb1e47d96efba14 Mon Sep 17 00:00:00 2001 From: James Harris Date: Tue, 2 Mar 2021 13:22:52 -0800 Subject: [PATCH] kernel: fix race conditions with z_ready_thread Several internal APIs wrote thread attributes (return value, mainly) _after_ calling `z_ready_thread`. This is unsafe, at least in SMP, because another core could have already picked up and run the thread. Fixes #32800. Signed-off-by: James Harris --- kernel/condvar.c | 2 +- kernel/futex.c | 4 ++-- kernel/stack.c | 4 ++-- kernel/timer.c | 4 ++-- lib/posix/pthread_mutex.c | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/kernel/condvar.c b/kernel/condvar.c index aa1b89ab3b8..861e5f0bbda 100644 --- a/kernel/condvar.c +++ b/kernel/condvar.c @@ -64,9 +64,9 @@ int z_impl_k_condvar_broadcast(struct k_condvar *condvar) /* wake up any threads that are waiting to write */ while ((pending_thread = z_unpend_first_thread(&condvar->wait_q)) != NULL) { + woken++; arch_thread_return_value_set(pending_thread, 0); z_ready_thread(pending_thread); - woken++; } z_reschedule(&lock, key); diff --git a/kernel/futex.c b/kernel/futex.c index c52f90d655d..2f87e3199a0 100644 --- a/kernel/futex.c +++ b/kernel/futex.c @@ -41,9 +41,9 @@ int z_impl_k_futex_wake(struct k_futex *futex, bool wake_all) do { thread = z_unpend_first_thread(&futex_data->wait_q); if (thread) { - z_ready_thread(thread); - arch_thread_return_value_set(thread, 0); woken++; + arch_thread_return_value_set(thread, 0); + z_ready_thread(thread); } } while (thread && wake_all); diff --git a/kernel/stack.c b/kernel/stack.c index 75f15026c15..2c96f002dc8 100644 --- a/kernel/stack.c +++ b/kernel/stack.c @@ -108,10 +108,10 @@ int z_impl_k_stack_push(struct k_stack *stack, stack_data_t data) first_pending_thread = z_unpend_first_thread(&stack->wait_q); if (first_pending_thread != NULL) { - z_ready_thread(first_pending_thread); - z_thread_return_value_set_with_data(first_pending_thread, 0, (void *)data); + + z_ready_thread(first_pending_thread); z_reschedule(&stack->lock, key); goto end; } else { diff --git a/kernel/timer.c b/kernel/timer.c index 42e6b10a0bf..b88c5dcc4e6 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -82,9 +82,9 @@ void z_timer_expiration_handler(struct _timeout *t) */ z_unpend_thread_no_timeout(thread); - z_ready_thread(thread); - arch_thread_return_value_set(thread, 0); + + z_ready_thread(thread); } diff --git a/lib/posix/pthread_mutex.c b/lib/posix/pthread_mutex.c index f1371848f3f..3e541b27ecb 100644 --- a/lib/posix/pthread_mutex.c +++ b/lib/posix/pthread_mutex.c @@ -142,8 +142,8 @@ int pthread_mutex_unlock(pthread_mutex_t *m) if (thread) { m->owner = (pthread_t)thread; m->lock_count++; - z_ready_thread(thread); arch_thread_return_value_set(thread, 0); + z_ready_thread(thread); z_reschedule_irqlock(key); return 0; }