From 687f799b7f9968780f4d3a4caec6999542248a00 Mon Sep 17 00:00:00 2001 From: iva kik Date: Wed, 21 Oct 2020 18:07:57 -0700 Subject: [PATCH] kernel/queue: fix queue append/get race k_queue_append and k_queue_alloc_append are in a race condition. Scenario: Using either of the above mentioned functions from a preemptive context, can lead to the element being appended to the queue to be lost. The element loss happens when the k_queue_append is interrupted at a critical point, when the list only contains one element and k_queue_get is called, before the k_queue_append is allowed to complete. Fix: Move sys_sflist_peek_tail inside queue_insert(). Add additional bool paramenter to queue_insert(), to indicate append/prepend operation. Fixes #29257 Signed-off-by: iva kik --- kernel/queue.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/kernel/queue.c b/kernel/queue.c index c10f8ea775e..e082080bc1c 100644 --- a/kernel/queue.c +++ b/kernel/queue.c @@ -136,11 +136,14 @@ static inline void z_vrfy_k_queue_cancel_wait(struct k_queue *queue) #endif static int32_t queue_insert(struct k_queue *queue, void *prev, void *data, - bool alloc) + bool alloc, bool is_append) { - k_spinlock_key_t key = k_spin_lock(&queue->lock); struct k_thread *first_pending_thread; + k_spinlock_key_t key = k_spin_lock(&queue->lock); + if (is_append) { + prev = sys_sflist_peek_tail(&queue->data_q); + } first_pending_thread = z_unpend_first_thread(&queue->wait_q); if (first_pending_thread != NULL) { @@ -173,29 +176,27 @@ static int32_t queue_insert(struct k_queue *queue, void *prev, void *data, void k_queue_insert(struct k_queue *queue, void *prev, void *data) { - (void)queue_insert(queue, prev, data, false); + (void)queue_insert(queue, prev, data, false, false); } void k_queue_append(struct k_queue *queue, void *data) { - (void)queue_insert(queue, sys_sflist_peek_tail(&queue->data_q), - data, false); + (void)queue_insert(queue, NULL, data, false, true); } void k_queue_prepend(struct k_queue *queue, void *data) { - (void)queue_insert(queue, NULL, data, false); + (void)queue_insert(queue, NULL, data, false, false); } int32_t z_impl_k_queue_alloc_append(struct k_queue *queue, void *data) { - return queue_insert(queue, sys_sflist_peek_tail(&queue->data_q), data, - true); + return queue_insert(queue, NULL, data, true, true); } #ifdef CONFIG_USERSPACE static inline int32_t z_vrfy_k_queue_alloc_append(struct k_queue *queue, - void *data) + void *data) { Z_OOPS(Z_SYSCALL_OBJ(queue, K_OBJ_QUEUE)); return z_impl_k_queue_alloc_append(queue, data); @@ -205,12 +206,13 @@ static inline int32_t z_vrfy_k_queue_alloc_append(struct k_queue *queue, int32_t z_impl_k_queue_alloc_prepend(struct k_queue *queue, void *data) { - return queue_insert(queue, NULL, data, true); + return queue_insert(queue, NULL, data, true, false); + } #ifdef CONFIG_USERSPACE static inline int32_t z_vrfy_k_queue_alloc_prepend(struct k_queue *queue, - void *data) + void *data) { Z_OOPS(Z_SYSCALL_OBJ(queue, K_OBJ_QUEUE)); return z_impl_k_queue_alloc_prepend(queue, data);