From 0bc342fbbd27fbe2b368ef1de92cbe9deadfc935 Mon Sep 17 00:00:00 2001 From: Armin Brauns Date: Mon, 8 May 2023 11:12:06 +0000 Subject: [PATCH] kernel: fix buffer overflow from incorrect K_MSGQ_DEFINE definition Without these parentheses, specifying a q_max_msgs of e.g. `MY_DEFAULT_QUEUESIZE+1` would result in a buffer of size (1 element + MY_DEFAULT_QUEUESIZE bytes). This would then lead to an unbounded buffer overflow because the queue never reaches the exact (offset by MY_DEFAULT_QUEUESIZE bytes) `buffer_end` and just keeps writing. Additionally, add asserts to make sure this can't happen again. Signed-off-by: Armin Brauns --- include/zephyr/kernel.h | 2 +- kernel/msg_q.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/include/zephyr/kernel.h b/include/zephyr/kernel.h index a89bdab0529..106b9dc70bd 100644 --- a/include/zephyr/kernel.h +++ b/include/zephyr/kernel.h @@ -4349,7 +4349,7 @@ struct k_msgq_attrs { _k_fifo_buf_##q_name[(q_max_msgs) * (q_msg_size)]; \ STRUCT_SECTION_ITERABLE(k_msgq, q_name) = \ Z_MSGQ_INITIALIZER(q_name, _k_fifo_buf_##q_name, \ - q_msg_size, q_max_msgs) + (q_msg_size), (q_max_msgs)) /** * @brief Initialize a message queue. diff --git a/kernel/msg_q.c b/kernel/msg_q.c index 6b68f7ac37d..1179612ce42 100644 --- a/kernel/msg_q.c +++ b/kernel/msg_q.c @@ -141,6 +141,8 @@ int z_impl_k_msgq_put(struct k_msgq *msgq, const void *data, k_timeout_t timeout return 0; } else { /* put message in queue */ + __ASSERT_NO_MSG(msgq->write_ptr >= msgq->buffer_start && + msgq->write_ptr < msgq->buffer_end); (void)memcpy(msgq->write_ptr, data, msgq->msg_size); msgq->write_ptr += msgq->msg_size; if (msgq->write_ptr == msgq->buffer_end) { @@ -230,6 +232,8 @@ int z_impl_k_msgq_get(struct k_msgq *msgq, void *data, k_timeout_t timeout) SYS_PORT_TRACING_OBJ_FUNC_BLOCKING(k_msgq, get, msgq, timeout); /* add thread's message to queue */ + __ASSERT_NO_MSG(msgq->write_ptr >= msgq->buffer_start && + msgq->write_ptr < msgq->buffer_end); (void)memcpy(msgq->write_ptr, pending_thread->base.swap_data, msgq->msg_size); msgq->write_ptr += msgq->msg_size;