diff --git a/components/esp32/ld/esp32.rom.ld b/components/esp32/ld/esp32.rom.ld index ccd9851a5..6058a9e61 100644 --- a/components/esp32/ld/esp32.rom.ld +++ b/components/esp32/ld/esp32.rom.ld @@ -34,7 +34,6 @@ PROVIDE ( cache_sram_mmu_set_rom = 0x400097f4 ); /* This is static function, but can be used, not generated by script*/ PROVIDE ( calc_rtc_memory_crc = 0x40008170 ); PROVIDE ( calloc = 0x4000bee4 ); -PROVIDE ( _calloc_r = 0x4000bbf8 ); PROVIDE ( __clear_cache = 0x40063860 ); PROVIDE ( _close_r = 0x4000bd3c ); PROVIDE ( co_default_bdaddr = 0x3ffae704 ); diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index ad971c176..4f9c016a5 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -33,26 +33,21 @@ possible. This should optimize the amount of RAM accessible to the code without /* This takes a memory chunk in a region that can be addressed as both DRAM as well as IRAM. It will convert it to - IRAM in such a way that it can be later freed. It assumes both the address as wel as the length to be word-aligned. + IRAM in such a way that it can be later freed. It assumes both the address as well as the length to be word-aligned. It returns a region that's 1 word smaller than the region given because it stores the original Dram address there. - - In theory, we can also make this work by prepending a struct that looks similar to the block link struct used by the - heap allocator itself, which will allow inspection tools relying on any block returned from any sort of malloc to - have such a block in front of it, work. We may do this later, if/when there is demand for it. For now, a simple - pointer is used. */ IRAM_ATTR static void *dram_alloc_to_iram_addr(void *addr, size_t len) { - uint32_t dstart = (int)addr; //First word - uint32_t dend = ((int)addr) + len - 4; //Last word - assert(dstart >= SOC_DIRAM_DRAM_LOW); - assert(dend <= SOC_DIRAM_DRAM_HIGH); + uintptr_t dstart = (uintptr_t)addr; //First word + uintptr_t dend = dstart + len - 4; //Last word + assert(esp_ptr_in_diram_dram((void *)dstart)); + assert(esp_ptr_in_diram_dram((void *)dend)); assert((dstart & 3) == 0); assert((dend & 3) == 0); uint32_t istart = SOC_DIRAM_IRAM_LOW + (SOC_DIRAM_DRAM_HIGH - dend); uint32_t *iptr = (uint32_t *)istart; *iptr = dstart; - return (void *)(iptr + 1); + return iptr + 1; } bool heap_caps_match(const heap_t *heap, uint32_t caps) @@ -67,6 +62,12 @@ IRAM_ATTR void *heap_caps_malloc( size_t size, uint32_t caps ) { void *ret = NULL; + if (size > HEAP_SIZE_MAX) { + // Avoids int overflow when adding small numbers to size, or + // calculating 'end' from start+size, by limiting 'size' to the possible range + return NULL; + } + if (caps & MALLOC_CAP_EXEC) { //MALLOC_CAP_EXEC forces an alloc from IRAM. There is a region which has both this as well as the following //caps, but the following caps are not possible for IRAM. Thus, the combination is impossible and we return @@ -75,9 +76,17 @@ IRAM_ATTR void *heap_caps_malloc( size_t size, uint32_t caps ) if ((caps & MALLOC_CAP_8BIT) || (caps & MALLOC_CAP_DMA)) { return NULL; } - //If any, EXEC memory should be 32-bit aligned, so round up to the next multiple of 4. - size = (size + 3) & (~3); + + caps |= MALLOC_CAP_32BIT; // IRAM is 32-bit accessible RAM } + + if (caps & MALLOC_CAP_32BIT) { + /* 32-bit accessible RAM should allocated in 4 byte aligned sizes + * (Future versions of ESP-IDF should possibly fail if an invalid size is requested) + */ + size = (size + 3) & (~3); // int overflow checked above + } + for (int prio = 0; prio < SOC_MEMORY_TYPE_NO_PRIOS; prio++) { //Iterate over heaps and check capabilities at this priority heap_t *heap; @@ -90,13 +99,13 @@ IRAM_ATTR void *heap_caps_malloc( size_t size, uint32_t caps ) //doesn't cover, see if they're available in other prios. if ((get_all_caps(heap) & caps) == caps) { //This heap can satisfy all the requested capabilities. See if we can grab some memory using it. - if ((caps & MALLOC_CAP_EXEC) && heap->start >= SOC_DIRAM_DRAM_LOW && heap->start < SOC_DIRAM_DRAM_HIGH) { + if ((caps & MALLOC_CAP_EXEC) && esp_ptr_in_diram_dram((void *)heap->start)) { //This is special, insofar that what we're going to get back is a DRAM address. If so, //we need to 'invert' it (lowest address in DRAM == highest address in IRAM and vice-versa) and //add a pointer to the DRAM equivalent before the address we're going to return. - ret = multi_heap_malloc(heap->heap, size + 4); + ret = multi_heap_malloc(heap->heap, size + 4); // int overflow checked above if (ret != NULL) { - return dram_alloc_to_iram_addr(ret, size + 4); + return dram_alloc_to_iram_addr(ret, size + 4); // int overflow checked above } } else { //Just try to alloc, nothing special. @@ -243,13 +252,11 @@ IRAM_ATTR static heap_t *find_containing_heap(void *ptr ) IRAM_ATTR void heap_caps_free( void *ptr) { - intptr_t p = (intptr_t)ptr; - if (ptr == NULL) { return; } - if ((p >= SOC_DIRAM_IRAM_LOW) && (p <= SOC_DIRAM_IRAM_HIGH)) { + if (esp_ptr_in_diram_iram(ptr)) { //Memory allocated here is actually allocated in the DRAM alias region and //cannot be de-allocated as usual. dram_alloc_to_iram_addr stores a pointer to //the equivalent DRAM address, though; free that. @@ -273,6 +280,10 @@ IRAM_ATTR void *heap_caps_realloc( void *ptr, size_t size, int caps) return NULL; } + if (size > HEAP_SIZE_MAX) { + return NULL; + } + heap_t *heap = find_containing_heap(ptr); assert(heap != NULL && "realloc() pointer is outside heap areas"); @@ -305,12 +316,18 @@ IRAM_ATTR void *heap_caps_realloc( void *ptr, size_t size, int caps) IRAM_ATTR void *heap_caps_calloc( size_t n, size_t size, uint32_t caps) { - void *r; - r = heap_caps_malloc(n*size, caps); - if (r != NULL) { - bzero(r, n*size); + void *result; + size_t size_bytes; + + if (__builtin_mul_overflow(n, size, &size_bytes)) { + return NULL; } - return r; + + result = heap_caps_malloc(size_bytes, caps); + if (result != NULL) { + bzero(result, size_bytes); + } + return result; } size_t heap_caps_get_free_size( uint32_t caps ) diff --git a/components/heap/heap_caps_init.c b/components/heap/heap_caps_init.c index a2b66bfc7..65eb452d2 100644 --- a/components/heap/heap_caps_init.c +++ b/components/heap/heap_caps_init.c @@ -31,7 +31,9 @@ struct registered_heap_ll registered_heaps; static void register_heap(heap_t *region) { - region->heap = multi_heap_register((void *)region->start, region->end - region->start); + size_t heap_size = region->end - region->start; + assert(heap_size <= HEAP_SIZE_MAX); + region->heap = multi_heap_register((void *)region->start, heap_size); if (region->heap != NULL) { ESP_EARLY_LOGD(TAG, "New heap initialised at %p", region->heap); } diff --git a/components/heap/heap_private.h b/components/heap/heap_private.h index a8a0ac9fd..f5a61f327 100644 --- a/components/heap/heap_private.h +++ b/components/heap/heap_private.h @@ -28,6 +28,8 @@ extern "C" { for heap_caps_init.c to share heap information with heap_caps.c */ +#define HEAP_SIZE_MAX (SOC_MAX_CONTIGUOUS_RAM_SIZE) + /* Type for describing each registered heap */ typedef struct heap_t_ { uint32_t caps[SOC_MEMORY_TYPE_NO_PRIOS]; ///< Capabilities for the type of memory in this heap (as a prioritised set). Copied from soc_memory_types so it's in RAM not flash. diff --git a/components/heap/multi_heap_poisoning.c b/components/heap/multi_heap_poisoning.c index aa50bc59e..3c8cff241 100644 --- a/components/heap/multi_heap_poisoning.c +++ b/components/heap/multi_heap_poisoning.c @@ -175,6 +175,9 @@ static bool verify_fill_pattern(void *data, size_t size, bool print_errors, bool void *multi_heap_malloc(multi_heap_handle_t heap, size_t size) { + if(size > SIZE_MAX - POISON_OVERHEAD) { + return NULL; + } multi_heap_internal_lock(heap); poison_head_t *head = multi_heap_malloc_impl(heap, size + POISON_OVERHEAD); uint8_t *data = NULL; @@ -217,6 +220,9 @@ void *multi_heap_realloc(multi_heap_handle_t heap, void *p, size_t size) poison_head_t *new_head; void *result = NULL; + if(size > SIZE_MAX - POISON_OVERHEAD) { + return NULL; + } if (p == NULL) { return multi_heap_malloc(heap, size); } diff --git a/components/heap/test/test_malloc.c b/components/heap/test/test_malloc.c index 23eef9d8b..3b329e386 100644 --- a/components/heap/test/test_malloc.c +++ b/components/heap/test/test_malloc.c @@ -65,7 +65,6 @@ TEST_CASE("Malloc/overwrite, then free all available DRAM", "[heap]") TEST_ASSERT(m1==m2); } - #if CONFIG_SPIRAM_USE_MALLOC #if (CONFIG_SPIRAM_MALLOC_RESERVE_INTERNAL > 1024) @@ -87,4 +86,30 @@ TEST_CASE("Check if reserved DMA pool still can allocate even when malloc()'ed m } #endif -#endif \ No newline at end of file +#endif + +TEST_CASE("alloc overflows should all fail", "[heap]") +{ + /* allocates 8 bytes if size_t overflows */ + TEST_ASSERT_NULL(calloc(SIZE_MAX / 2 + 4, 2)); + + /* will overflow if any poisoning is enabled + (should fail for sensible OOM reasons, otherwise) */ + TEST_ASSERT_NULL(malloc(SIZE_MAX - 1)); + TEST_ASSERT_NULL(calloc(SIZE_MAX - 1, 1)); + + /* will overflow when the size is rounded up to word align it */ + TEST_ASSERT_NULL(heap_caps_malloc(SIZE_MAX-1, MALLOC_CAP_32BIT)); + + TEST_ASSERT_NULL(heap_caps_malloc(SIZE_MAX-1, MALLOC_CAP_EXEC)); +} + +TEST_CASE("unreasonable allocs should all fail", "[heap]") +{ + TEST_ASSERT_NULL(calloc(16, 1024*1024)); + TEST_ASSERT_NULL(malloc(16*1024*1024)); + TEST_ASSERT_NULL(malloc(SIZE_MAX / 2)); + TEST_ASSERT_NULL(malloc(SIZE_MAX - 256)); + TEST_ASSERT_NULL(malloc(xPortGetFreeHeapSize() - 1)); +} + diff --git a/components/newlib/syscalls.c b/components/newlib/syscalls.c index f4528f6ea..d3563e247 100644 --- a/components/newlib/syscalls.c +++ b/components/newlib/syscalls.c @@ -47,11 +47,17 @@ void* IRAM_ATTR _realloc_r(struct _reent *r, void* ptr, size_t size) return heap_caps_realloc_default( ptr, size ); } -void* IRAM_ATTR _calloc_r(struct _reent *r, size_t count, size_t size) +void* IRAM_ATTR _calloc_r(struct _reent *r, size_t nmemb, size_t size) { - void* result = heap_caps_malloc_default(count * size); - if (result) { - bzero(result, count * size); + void *result; + size_t size_bytes; + if (__builtin_mul_overflow(nmemb, size, &size_bytes)) { + return NULL; + } + + result = malloc(size_bytes); + if (result != NULL) { + bzero(result, size_bytes); } return result; } diff --git a/components/soc/esp32/include/soc/soc.h b/components/soc/esp32/include/soc/soc.h index 86a94c974..41947847a 100644 --- a/components/soc/esp32/include/soc/soc.h +++ b/components/soc/esp32/include/soc/soc.h @@ -63,6 +63,8 @@ #define SOC_IROM_HIGH 0x40400000 #define SOC_DROM_LOW 0x3F400000 #define SOC_DROM_HIGH 0x3F800000 +#define SOC_DRAM_LOW 0x3FAE0000 +#define SOC_DRAM_HIGH 0x40000000 #define SOC_RTC_IRAM_LOW 0x400C0000 #define SOC_RTC_IRAM_HIGH 0x400C2000 #define SOC_RTC_DATA_LOW 0x50000000 @@ -70,6 +72,12 @@ #define SOC_EXTRAM_DATA_LOW 0x3F800000 #define SOC_EXTRAM_DATA_HIGH 0x3FC00000 +#define SOC_MAX_CONTIGUOUS_RAM_SIZE 0x400000 ///< Largest span of contiguous memory (DRAM or IRAM) in the address space + +#define SOC_CACHE_PRO_LOW 0x40070000 +#define SOC_CACHE_PRO_HIGH 0x40078000 +#define SOC_CACHE_APP_LOW 0x40078000 +#define SOC_CACHE_APP_HIGH 0x40080000 #define DR_REG_DPORT_BASE 0x3ff00000 #define DR_REG_AES_BASE 0x3ff01000 diff --git a/components/soc/include/soc/soc_memory_layout.h b/components/soc/include/soc/soc_memory_layout.h index 6273b1dbc..593d68d9e 100644 --- a/components/soc/include/soc/soc_memory_layout.h +++ b/components/soc/include/soc/soc_memory_layout.h @@ -94,3 +94,28 @@ inline static bool IRAM_ATTR esp_ptr_internal(const void *p) { inline static bool IRAM_ATTR esp_ptr_external_ram(const void *p) { return ((intptr_t)p >= SOC_EXTRAM_DATA_LOW && (intptr_t)p < SOC_EXTRAM_DATA_HIGH); } + +inline static bool IRAM_ATTR esp_ptr_in_iram(const void *p) { +#ifndef CONFIG_FREERTOS_UNICORE + return ((intptr_t)p >= SOC_IRAM_LOW && (intptr_t)p < SOC_IRAM_HIGH); +#else + return ((intptr_t)p >= SOC_CACHE_APP_LOW && (intptr_t)p < SOC_IRAM_HIGH); +#endif +} + +inline static bool IRAM_ATTR esp_ptr_in_drom(const void *p) { + return ((intptr_t)p >= SOC_DROM_LOW && (intptr_t)p < SOC_DROM_HIGH); +} + +inline static bool IRAM_ATTR esp_ptr_in_dram(const void *p) { + return ((intptr_t)p >= SOC_DRAM_LOW && (intptr_t)p < SOC_DRAM_HIGH); +} + +inline static bool IRAM_ATTR esp_ptr_in_diram_dram(const void *p) { + return ((intptr_t)p >= SOC_DIRAM_DRAM_LOW && (intptr_t)p < SOC_DIRAM_DRAM_HIGH); +} + +inline static bool IRAM_ATTR esp_ptr_in_diram_iram(const void *p) { + return ((intptr_t)p >= SOC_DIRAM_IRAM_LOW && (intptr_t)p < SOC_DIRAM_IRAM_HIGH); +} +