Mirrors/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2026-10-01 18:50:34 +03:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'fix/ringbuf_max_item_size_v5.5' into 'release/v5.5'

Browse Source 
fix(esp_ringbuf): harden ringbuf creation sizes checks (v5.5)

See merge request espressif/esp-idf!51001
This commit is contained in:
Jiang Jiang Jian
2026-09-07 11:26:54 +08:00
parent 362a1776ec 4813be4c2c
commit 290bea2cc6
3 changed files with 143 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
components/esp_ringbuf/include/freertos/ringbuf.h
+10 -1
View File
@@ -73,6 +73,7 @@ typedef struct xSTATIC_RINGBUFFER {
* @param[in] xBufferType Type of ring buffer, see documentation.
*
* @note xBufferSize of no-split/allow-split buffers will be rounded up to the nearest 32-bit aligned size.
* The aligned size must be large enough to hold two item headers.
*
* @return A handle to the created ring buffer, or NULL in case of error.
*/
@@ -87,6 +88,9 @@ RingbufHandle_t xRingbufferCreate(size_t xBufferSize, RingbufferType_t xBufferTy
* @param[in] xItemSize Size of each item to be put into the ring buffer
* @param[in] xItemNum Maximum number of items the buffer needs to hold simultaneously
*
* @note Returns NULL if xItemNum is 0, the calculated buffer size overflows,
* or the resulting buffer is too small to hold two item headers.
*
* @return A RingbufHandle_t handle to the created ring buffer, or NULL in case of error.
*/
RingbufHandle_t xRingbufferCreateNoSplit(size_t xItemSize, size_t xItemNum);
@@ -102,8 +106,9 @@ RingbufHandle_t xRingbufferCreateNoSplit(size_t xItemSize, size_t xItemNum);
* which will be used to hold the ring buffer's data structure
*
* @note xBufferSize of no-split/allow-split buffers MUST be 32-bit aligned.
* The aligned size must be large enough to hold two item headers.
*
* @return A handle to the created ring buffer
* @return A handle to the created ring buffer, or NULL in case of error.
*/
RingbufHandle_t xRingbufferCreateStatic(size_t xBufferSize,
RingbufferType_t xBufferType,
@@ -528,6 +533,10 @@ BaseType_t xRingbufferGetStaticBuffer(RingbufHandle_t xRingbuffer, uint8_t **ppu
*
* @note A queue created using this function must only be deleted using
* vRingbufferDeleteWithCaps()
* @note xBufferSize of no-split/allow-split buffers will be rounded up to the
* nearest 32-bit aligned size. The aligned size must be large enough to hold
* two item headers.
*
* @param[in] xBufferSize Size of the buffer in bytes
* @param[in] xBufferType Type of ring buffer, see documentation.
* @param[in] uxMemoryCaps Memory capabilities of the queue's memory (see
components/esp_ringbuf/ringbuf.c
+52 -8
View File
@@ -91,6 +91,11 @@ static void prvInitializeNewRingbuffer(size_t xBufferSize,
Ringbuffer_t *pxNewRingbuffer,
uint8_t *pucRingbufferStorage);
//Validate and align buffer size for dynamically allocated ring buffers
static BaseType_t prvGetAlignedBufferSize(size_t xBufferSize,
RingbufferType_t xBufferType,
size_t *pxAlignedBufferSize);
//Calculate current amount of free space (in bytes) in the ring buffer
static size_t prvGetFreeSize(Ringbuffer_t *pxRingbuffer);
@@ -204,6 +209,29 @@ static BaseType_t prvReceiveGenericFromISR(Ringbuffer_t *pxRingbuffer,
// ------------------------------------------------ Static Functions ---------------------------------------------------
static BaseType_t prvGetAlignedBufferSize(size_t xBufferSize,
RingbufferType_t xBufferType,
size_t *pxAlignedBufferSize)
{
if (xBufferType >= RINGBUF_TYPE_MAX || xBufferSize == 0) {
return pdFALSE;
}
//No-split/allow-split buffers must be large enough to avoid underflowing xMaxItemSize
if (xBufferType != RINGBUF_TYPE_BYTEBUF) {
if (xBufferSize > SIZE_MAX - rbALIGN_MASK) {
return pdFALSE; //Alignment would overflow
}
xBufferSize = rbALIGN_SIZE(xBufferSize);
if (xBufferSize < rbHEADER_SIZE * 2) {
return pdFALSE;
}
}
*pxAlignedBufferSize = xBufferSize;
return pdTRUE;
}
static void prvInitializeNewRingbuffer(size_t xBufferSize,
RingbufferType_t xBufferType,
Ringbuffer_t *pxNewRingbuffer,
@@ -944,10 +972,11 @@ RingbufHandle_t xRingbufferCreate(size_t xBufferSize, RingbufferType_t xBufferTy
configASSERT(xBufferSize > 0);
configASSERT(xBufferType < RINGBUF_TYPE_MAX);
//Allocate memory
if (xBufferType != RINGBUF_TYPE_BYTEBUF) {
xBufferSize = rbALIGN_SIZE(xBufferSize); //xBufferSize is rounded up for no-split/allow-split buffers
if (prvGetAlignedBufferSize(xBufferSize, xBufferType, &xBufferSize) != pdTRUE) {
return NULL;
}
//Allocate memory
Ringbuffer_t *pxNewRingbuffer = calloc(1, sizeof(Ringbuffer_t));
uint8_t *pucRingbufferStorage = malloc(xBufferSize);
if (pxNewRingbuffer == NULL || pucRingbufferStorage == NULL) {
@@ -966,7 +995,16 @@ err:
RingbufHandle_t xRingbufferCreateNoSplit(size_t xItemSize, size_t xItemNum)
{
return xRingbufferCreate((rbALIGN_SIZE(xItemSize) + rbHEADER_SIZE) * xItemNum, RINGBUF_TYPE_NOSPLIT);
//Guard the (aligned item size + header) * item count computation against overflow
size_t xItemSizeWithHeader;
size_t xBufferSize;
if (xItemNum == 0 || xItemSize > SIZE_MAX - rbALIGN_MASK ||
__builtin_add_overflow(rbALIGN_SIZE(xItemSize), rbHEADER_SIZE, &xItemSizeWithHeader) ||
__builtin_mul_overflow(xItemSizeWithHeader, xItemNum, &xBufferSize)) {
return NULL;
}
return xRingbufferCreate(xBufferSize, RINGBUF_TYPE_NOSPLIT);
}
RingbufHandle_t xRingbufferCreateStatic(size_t xBufferSize,
@@ -978,9 +1016,15 @@ RingbufHandle_t xRingbufferCreateStatic(size_t xBufferSize,
configASSERT(xBufferSize > 0);
configASSERT(xBufferType < RINGBUF_TYPE_MAX);
configASSERT(pucRingbufferStorage != NULL && pxStaticRingbuffer != NULL);
if (xBufferType >= RINGBUF_TYPE_MAX || xBufferSize == 0) {
return NULL;
}
if (xBufferType != RINGBUF_TYPE_BYTEBUF) {
//No-split/allow-split buffer sizes must be 32-bit aligned
//No-split/allow-split buffer sizes must be 32-bit aligned and large enough to avoid underflowing xMaxItemSize
configASSERT(rbCHECK_ALIGNED(xBufferSize));
if (!rbCHECK_ALIGNED(xBufferSize) || xBufferSize < rbHEADER_SIZE * 2) {
return NULL;
}
}
Ringbuffer_t *pxNewRingbuffer = (Ringbuffer_t *)pxStaticRingbuffer;
@@ -1464,11 +1508,11 @@ RingbufHandle_t xRingbufferCreateWithCaps(size_t xBufferSize, RingbufferType_t x
StaticRingbuffer_t *pxStaticRingbuffer;
uint8_t *pucRingbufferStorage;
//Allocate memory
if (xBufferType != RINGBUF_TYPE_BYTEBUF) {
xBufferSize = rbALIGN_SIZE(xBufferSize); //xBufferSize is rounded up for no-split/allow-split buffers
if (prvGetAlignedBufferSize(xBufferSize, xBufferType, &xBufferSize) != pdTRUE) {
return NULL;
}
//Allocate memory
pxStaticRingbuffer = heap_caps_malloc(sizeof(StaticRingbuffer_t), (uint32_t)uxMemoryCaps);
pucRingbufferStorage = heap_caps_malloc(xBufferSize, (uint32_t)uxMemoryCaps);
components/esp_ringbuf/test_apps/main/test_ringbuf_common.c
+81
View File
@@ -10,6 +10,7 @@
*/
#include "sdkconfig.h"
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -19,6 +20,7 @@
#include "freertos/semphr.h"
#include "freertos/ringbuf.h"
#include "unity.h"
#include "esp_heap_caps.h"
#include "esp_rom_sys.h"
#include "test_functions.h"
@@ -176,6 +178,85 @@ void receive_check_and_return_item_byte_buffer(RingbufHandle_t handle, const uin
}
}
static void check_too_small_ringbuffer_send_rejected(RingbufferType_t buffer_type)
{
const size_t too_small_size = ITEM_HDR_SIZE;
TEST_ASSERT_TRUE(heap_caps_check_integrity_all(true));
RingbufHandle_t handle = xRingbufferCreate(too_small_size, buffer_type);
BaseType_t ret = pdFALSE;
if (handle != NULL) {
if (buffer_type == RINGBUF_TYPE_NOSPLIT) {
void *item = NULL;
ret = xRingbufferSendAcquire(handle, &item, SMALL_ITEM_SIZE, 0);
if (ret == pdTRUE) {
memcpy(item, small_item, SMALL_ITEM_SIZE);
}
} else {
ret = xRingbufferSend(handle, small_item, SMALL_ITEM_SIZE, 0);
}
}
TEST_ASSERT_TRUE(heap_caps_check_integrity_all(true));
TEST_ASSERT_EQUAL(pdFALSE, ret);
if (handle != NULL && ret == pdFALSE) {
vRingbufferDelete(handle);
}
}
static void check_too_small_static_ringbuffer_rejected(RingbufferType_t buffer_type)
{
static StaticRingbuffer_t ringbuffer_struct;
//Aligned so the no-split/allow-split alignment assert is satisfied and only the size guard rejects
static uint8_t ringbuffer_storage[ITEM_HDR_SIZE] __attribute__((aligned(sizeof(size_t))));
TEST_ASSERT_TRUE(heap_caps_check_integrity_all(true));
RingbufHandle_t handle = xRingbufferCreateStatic(sizeof(ringbuffer_storage), buffer_type,
ringbuffer_storage, &ringbuffer_struct);
TEST_ASSERT_TRUE(heap_caps_check_integrity_all(true));
TEST_ASSERT_NULL(handle);
}
static void check_too_small_ringbuffer_with_caps_rejected(RingbufferType_t buffer_type)
{
const size_t too_small_size = ITEM_HDR_SIZE;
TEST_ASSERT_TRUE(heap_caps_check_integrity_all(true));
//The size guard rejects before any allocation, so the specific caps are irrelevant here
RingbufHandle_t handle = xRingbufferCreateWithCaps(too_small_size, buffer_type, MALLOC_CAP_8BIT);
TEST_ASSERT_TRUE(heap_caps_check_integrity_all(true));
TEST_ASSERT_NULL(handle);
}
TEST_CASE("Ringbuffer prevents heap corruption when size can underflow max item size", "[esp_ringbuf]")
{
check_too_small_ringbuffer_send_rejected(RINGBUF_TYPE_NOSPLIT);
check_too_small_ringbuffer_send_rejected(RINGBUF_TYPE_ALLOWSPLIT);
check_too_small_static_ringbuffer_rejected(RINGBUF_TYPE_NOSPLIT);
check_too_small_static_ringbuffer_rejected(RINGBUF_TYPE_ALLOWSPLIT);
check_too_small_ringbuffer_with_caps_rejected(RINGBUF_TYPE_NOSPLIT);
check_too_small_ringbuffer_with_caps_rejected(RINGBUF_TYPE_ALLOWSPLIT);
RingbufHandle_t byte_buffer = xRingbufferCreate(ITEM_HDR_SIZE, RINGBUF_TYPE_BYTEBUF);
TEST_ASSERT_NOT_NULL(byte_buffer);
vRingbufferDelete(byte_buffer);
}
TEST_CASE("Ringbuffer fixed-size no-split creation rejects size overflow", "[esp_ringbuf][linux]")
{
const size_t overflowing_item_count = (SIZE_MAX / SMALL_ITEM_SIZE) + 2;
TEST_ASSERT_NULL(xRingbufferCreateNoSplit(SMALL_ITEM_SIZE, overflowing_item_count));
}
/* ----------------- Basic ring buffer behavior tests cases --------------------
* The following set of test cases will test basic send, receive, wrap around and buffer full
* behavior of each type of ring buffer.