Skip to content

Commit

Permalink
Fix buffer overflow in the FreeRTOS compat queue.
Browse files Browse the repository at this point in the history
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 <hugo.lefeuvre@scisemi.com>
  • Loading branch information
hlef authored and davidchisnall committed May 13, 2024
1 parent 75ed409 commit c6bcc8c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
2 changes: 1 addition & 1 deletion sdk/include/FreeRTOS-Compat/queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 17 additions & 0 deletions tests/queue-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <cstdlib>
#define TEST_NAME "Queue"
#include "tests.hh"
#include <FreeRTOS-Compat/queue.h>
#include <debug.hh>
#include <errno.h>
#include <queue.h>
Expand Down Expand Up @@ -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");
}

0 comments on commit c6bcc8c

Please sign in to comment.