From f7eecfcc6734b5bace938b0bc319608a4a5c81b8 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 9 Feb 2018 11:41:27 +0800 Subject: [PATCH] heap: Fix bug when realloc moves data between heaps When realloc-ing to a smaller buffer size which ends up allocated in a different heap, the heap structure is corrupted. This can only happen: * If heap checking is Comprehensive (meaning buffers are never shrunk in place) and the heap the buffer was originally allocated in is full. * Calling heap_caps_realloc() to deliberately move a buffer to a different capabilities type, and shrink it at the same time. Probable fix for https://github.com/espressif/esp-idf/issues/1582 Probably the same issue: https://www.esp32.com/viewtopic.php?f=2&t=4583 https://www.esp32.com/viewtopic.php?f=13&t=3717 --- components/heap/heap_caps.c | 2 +- components/heap/test/test_realloc.c | 50 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 components/heap/test/test_realloc.c diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index 775a79a9f..31cfa8d55 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -309,7 +309,7 @@ IRAM_ATTR void *heap_caps_realloc( void *ptr, size_t size, int caps) if (new_p != NULL) { size_t old_size = multi_heap_get_allocated_size(heap->heap, ptr); assert(old_size > 0); - memcpy(new_p, ptr, old_size); + memcpy(new_p, ptr, MIN(size, old_size)); heap_caps_free(ptr); return new_p; } diff --git a/components/heap/test/test_realloc.c b/components/heap/test/test_realloc.c new file mode 100644 index 000000000..290ee3da6 --- /dev/null +++ b/components/heap/test/test_realloc.c @@ -0,0 +1,50 @@ +/* + Generic test for realloc +*/ + +#include +#include +#include "unity.h" +#include "sdkconfig.h" +#include "esp_heap_caps.h" + + +#ifndef CONFIG_HEAP_POISONING_COMPREHENSIVE +/* (can't realloc in place if comprehensive is enabled) */ + +TEST_CASE("realloc shrink buffer in place", "[heap]") +{ + void *x = malloc(64); + TEST_ASSERT(x); + void *y = realloc(p, 48); + TEST_ASSERT_EQUAL_PTR(x, y); +} + +#endif + +TEST_CASE("realloc move data to a new heap type", "[heap]") +{ + const char *test = "I am some test content to put in the heap"; + char buf[64]; + memset(buf, 0xEE, 64); + strlcpy(buf, test, 64); + + char *a = malloc(64); + memcpy(a, buf, 64); + + // move data from 'a' to IRAM + char *b = heap_caps_realloc(a, 64, MALLOC_CAP_EXEC); + TEST_ASSERT_NOT_NULL(b); + TEST_ASSERT_NOT_EQUAL(a, b); + TEST_ASSERT(heap_caps_check_integrity(MALLOC_CAP_INVALID, true)); + TEST_ASSERT_EQUAL_HEX32_ARRAY(buf, b, 64/sizeof(uint32_t)); + + // Move data back to DRAM + char *c = heap_caps_realloc(b, 48, MALLOC_CAP_8BIT); + TEST_ASSERT_NOT_NULL(c); + TEST_ASSERT_NOT_EQUAL(b, c); + TEST_ASSERT(heap_caps_check_integrity(MALLOC_CAP_INVALID, true)); + TEST_ASSERT_EQUAL_HEX8_ARRAY(buf, c, 48); + + free(c); +}