From c6bcc8ca191e82a004d30371c943b020ae90a788 Mon Sep 17 00:00:00 2001 From: Hugo Lefeuvre Date: Mon, 13 May 2024 10:06:14 +0200 Subject: [PATCH] Fix buffer overflow in the FreeRTOS compat queue. Our allocation size only covers the first field of `QueueHandle_t` (`struct QueueHandle`), and the `void * freePointer` is out of bounds. This may be due to unfortunate naming of the two data types - I would typically expect `QueueHandle_t` to be an alias for `struct QueueHandle` but that is not the case here. Unfortunately this has not been detected earlier because the Arty board (unlike the Ibex simulator) does not seem to trigger a tag violation. This will be investigated separately. For now, add a test that highlights the bug in the Ibex simulator, and fix the bug. The test is intentionally left very simple, we can extend later on if needed. Signed-off-by: Hugo Lefeuvre --- sdk/include/FreeRTOS-Compat/queue.h | 2 +- tests/queue-test.cc | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/sdk/include/FreeRTOS-Compat/queue.h b/sdk/include/FreeRTOS-Compat/queue.h index 25c9e259..088d5880 100644 --- a/sdk/include/FreeRTOS-Compat/queue.h +++ b/sdk/include/FreeRTOS-Compat/queue.h @@ -88,7 +88,7 @@ static inline QueueHandle_t xQueueCreate(UBaseType_t uxQueueLength, { QueueHandle_t ret; struct Timeout timeout = {0, UnlimitedTimeout}; - ret = (QueueHandle_t)malloc(sizeof(struct QueueHandle)); + ret = (QueueHandle_t)malloc(sizeof(*ret)); if (!ret) { return NULL; diff --git a/tests/queue-test.cc b/tests/queue-test.cc index 6d49f289..3b1fd052 100644 --- a/tests/queue-test.cc +++ b/tests/queue-test.cc @@ -6,6 +6,7 @@ #include #define TEST_NAME "Queue" #include "tests.hh" +#include #include #include #include @@ -169,9 +170,25 @@ void test_queue_sealed() debug_log("All queue compartment tests successful"); } +void test_queue_freertos() +{ + debug_log("Testing FreeRTOS queues"); + auto quotaBegin = heap_quota_remaining(MALLOC_CAPABILITY); + auto freertosQueue = xQueueCreate(10, sizeof(int)); + vQueueDelete(freertosQueue); + auto quotaEnd = heap_quota_remaining(MALLOC_CAPABILITY); + TEST( + quotaBegin == quotaEnd, + "The FreeRTOS queue wrapper leaks memory: quota before is {}, after {}", + quotaBegin, + quotaEnd); + debug_log("All FreeRTOS queue tests successful"); +} + void test_queue() { test_queue_unsealed(); test_queue_sealed(); + test_queue_freertos(); debug_log("All queue tests successful"); }