From da9d8a143a4047e029de9a3d0f82f544b2353b66 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Mon, 11 Nov 2024 07:48:49 +0100 Subject: [PATCH] feat(heap): Report prohibited usage of MALLOC_CAPS_EXEC at compile time Add a condition on the definition of the MALLOC_CAP_EXEC macro to prevent it from being defined if ESP_SYSTEM_MEMPROT_FEATURE or ESP_SYSTEM_PMP_IDRAM_SPLIT is enabled, thus throwing a compile time error when using it. Closes https://github.com/espressif/esp-idf/issues/14837 --- components/esp_system/Kconfig | 2 ++ components/heap/heap_caps_base.c | 9 +++++++-- components/heap/include/esp_heap_caps.h | 2 ++ components/heap/port/esp32s3/memory_layout.c | 4 +++- .../test_apps/heap_tests/main/test_aligned_alloc_caps.c | 4 ---- components/heap/test_apps/heap_tests/main/test_diram.c | 4 ++-- components/heap/test_apps/heap_tests/main/test_malloc.c | 2 ++ .../heap/test_apps/heap_tests/main/test_malloc_caps.c | 7 ++----- docs/en/migration-guides/release-6.x/6.0/system.rst | 7 +++++++ tools/idf_py_actions/hints.yml | 4 ++++ 10 files changed, 31 insertions(+), 14 deletions(-) diff --git a/components/esp_system/Kconfig b/components/esp_system/Kconfig index d8e7f4731b..b06b1b940d 100644 --- a/components/esp_system/Kconfig +++ b/components/esp_system/Kconfig @@ -168,6 +168,8 @@ menu "ESP System Settings" (above the splitting address). The memory protection is effective on all access through the IRAM0 and DRAM0 buses. + Note: allocating memory with MALLOC_CAP_EXEC capability is not possible when this config is enabled. + choice ESP_SYSTEM_MEMPROT_MODE prompt "Memory Protection configurations" depends on ESP_SYSTEM_MEMPROT diff --git a/components/heap/heap_caps_base.c b/components/heap/heap_caps_base.c index 0280b85bd5..f6e8b5bdd1 100644 --- a/components/heap/heap_caps_base.c +++ b/components/heap/heap_caps_base.c @@ -116,7 +116,9 @@ HEAP_IRAM_ATTR NOINLINE_ATTR void *heap_caps_aligned_alloc_base(size_t alignment return NULL; } - if (caps & MALLOC_CAP_EXEC) { +#if !(CONFIG_ESP_SYSTEM_MEMPROT_FEATURE || CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT) + const bool exec_in_caps = caps & MALLOC_CAP_EXEC; + if (exec_in_caps) { //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 //NULL directly, even although our heap capabilities (based on soc_memory_tags & soc_memory_regions) would @@ -126,6 +128,9 @@ HEAP_IRAM_ATTR NOINLINE_ATTR void *heap_caps_aligned_alloc_base(size_t alignment } caps |= MALLOC_CAP_32BIT; // IRAM is 32-bit accessible RAM } +#else + const bool exec_in_caps = false; +#endif if (caps & MALLOC_CAP_32BIT) { /* 32-bit accessible RAM should allocated in 4 byte aligned sizes @@ -148,7 +153,7 @@ HEAP_IRAM_ATTR NOINLINE_ATTR void *heap_caps_aligned_alloc_base(size_t alignment //This heap can satisfy all the requested capabilities. See if we can grab some memory using it. // If MALLOC_CAP_EXEC is requested but the DRAM and IRAM are on the same addresses (like on esp32c6) // proceed as for a default allocation. - if ((caps & MALLOC_CAP_EXEC) && + if (exec_in_caps && ((!esp_dram_match_iram() && esp_ptr_in_diram_dram((void *)heap->start)) || (!esp_rtc_dram_match_rtc_iram() && esp_ptr_in_rtc_dram_fast((void *)heap->start)))) { //This is special, insofar that what we're going to get back is a DRAM address. If so, diff --git a/components/heap/include/esp_heap_caps.h b/components/heap/include/esp_heap_caps.h index c5aaed2fbc..0b73d9a84e 100644 --- a/components/heap/include/esp_heap_caps.h +++ b/components/heap/include/esp_heap_caps.h @@ -26,7 +26,9 @@ extern "C" { /** * @brief Flags to indicate the capabilities of the various memory systems */ +#if !(CONFIG_ESP_SYSTEM_MEMPROT_FEATURE || CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT) #define MALLOC_CAP_EXEC (1<<0) ///< Memory must be able to run executable code +#endif #define MALLOC_CAP_32BIT (1<<1) ///< Memory must allow for aligned 32-bit data accesses #define MALLOC_CAP_8BIT (1<<2) ///< Memory must allow for 8/16/...-bit data accesses #define MALLOC_CAP_DMA (1<<3) ///< Memory must be able to accessed by DMA diff --git a/components/heap/port/esp32s3/memory_layout.c b/components/heap/port/esp32s3/memory_layout.c index 1d016252c2..f25d962cfa 100644 --- a/components/heap/port/esp32s3/memory_layout.c +++ b/components/heap/port/esp32s3/memory_layout.c @@ -44,9 +44,11 @@ enum { #ifdef CONFIG_ESP_SYSTEM_MEMPROT #define MALLOC_DIRAM_BASE_CAPS ESP32S3_MEM_COMMON_CAPS | MALLOC_CAP_INTERNAL | MALLOC_CAP_DMA | MALLOC_CAP_RETENTION #define MALLOC_RTCRAM_BASE_CAPS ESP32S3_MEM_COMMON_CAPS | MALLOC_CAP_INTERNAL +#define MALLOC_IRAM_BASE_CAPS 0 #else #define MALLOC_DIRAM_BASE_CAPS ESP32S3_MEM_COMMON_CAPS | MALLOC_CAP_INTERNAL | MALLOC_CAP_DMA | MALLOC_CAP_RETENTION | MALLOC_CAP_EXEC #define MALLOC_RTCRAM_BASE_CAPS ESP32S3_MEM_COMMON_CAPS | MALLOC_CAP_INTERNAL | MALLOC_CAP_EXEC +#define MALLOC_IRAM_BASE_CAPS MALLOC_CAP_EXEC #endif // The memory used for SIMD instructions requires the bus of its memory regions be able to transfer the data in 128-bit @@ -61,7 +63,7 @@ const soc_memory_type_desc_t soc_memory_types[SOC_MEMORY_TYPE_NUM] = { /* Mem Type Name | High Priority Matching | Medium Priority Matching | Low Priority Matching */ [SOC_MEMORY_TYPE_DIRAM] = { "RAM", { MALLOC_DIRAM_BASE_CAPS | MALLOC_CAP_SIMD, 0, 0 }}, [SOC_MEMORY_TYPE_DRAM] = { "DRAM", { 0, ESP32S3_MEM_COMMON_CAPS | MALLOC_CAP_INTERNAL | MALLOC_CAP_DMA | MALLOC_CAP_SIMD, 0 }}, - [SOC_MEMORY_TYPE_IRAM] = { "IRAM", { MALLOC_CAP_EXEC, MALLOC_CAP_32BIT | MALLOC_CAP_INTERNAL, 0 }}, + [SOC_MEMORY_TYPE_IRAM] = { "IRAM", { MALLOC_IRAM_BASE_CAPS, MALLOC_CAP_32BIT | MALLOC_CAP_INTERNAL, 0 }}, [SOC_MEMORY_TYPE_SPIRAM] = { "SPIRAM", { MALLOC_CAP_SPIRAM, 0, ESP32S3_MEM_COMMON_CAPS | MALLOC_CAP_SIMD }}, [SOC_MEMORY_TYPE_RTCRAM] = { "RTCRAM", { MALLOC_CAP_RTCRAM, 0, MALLOC_RTCRAM_BASE_CAPS }}, }; diff --git a/components/heap/test_apps/heap_tests/main/test_aligned_alloc_caps.c b/components/heap/test_apps/heap_tests/main/test_aligned_alloc_caps.c index 9ff98e5408..4c7e1fbf1f 100644 --- a/components/heap/test_apps/heap_tests/main/test_aligned_alloc_caps.c +++ b/components/heap/test_apps/heap_tests/main/test_aligned_alloc_caps.c @@ -45,10 +45,6 @@ TEST_CASE("Capabilities aligned allocator test", "[heap][psram]") } } - //Alloc from a non permitted area: - uint32_t *not_permitted_buf = (uint32_t *)heap_caps_aligned_alloc(alignments, (alignments + 137), MALLOC_CAP_EXEC | MALLOC_CAP_32BIT); - TEST_ASSERT( not_permitted_buf == NULL ); - #if CONFIG_SPIRAM alignments = 0; printf("[ALIGNED_ALLOC] Allocating from external memory: \n"); diff --git a/components/heap/test_apps/heap_tests/main/test_diram.c b/components/heap/test_apps/heap_tests/main/test_diram.c index cd4f53bcb7..39a1716f0c 100644 --- a/components/heap/test_apps/heap_tests/main/test_diram.c +++ b/components/heap/test_apps/heap_tests/main/test_diram.c @@ -15,7 +15,7 @@ #define ALLOC_SZ 1024 -#if !CONFIG_ESP_SYSTEM_MEMPROT +#if !(CONFIG_ESP_SYSTEM_MEMPROT_FEATURE || CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT) static void *malloc_block_diram(uint32_t caps) { void *attempts[256] = { 0 }; // Allocate up to 256 ALLOC_SZ blocks to exhaust all non-D/IRAM memory temporarily @@ -78,4 +78,4 @@ TEST_CASE("Allocate D/IRAM as IRAM", "[heap][qemu-ignore]") free(iram); } -#endif // !CONFIG_ESP_SYSTEM_MEMPROT +#endif // !(CONFIG_ESP_SYSTEM_MEMPROT_FEATURE || CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT) diff --git a/components/heap/test_apps/heap_tests/main/test_malloc.c b/components/heap/test_apps/heap_tests/main/test_malloc.c index 1289287e93..152cd22bdb 100644 --- a/components/heap/test_apps/heap_tests/main/test_malloc.c +++ b/components/heap/test_apps/heap_tests/main/test_malloc.c @@ -154,7 +154,9 @@ TEST_CASE("alloc overflows should all fail", "[heap]") /* will overflow when the size is rounded up to word align it */ TEST_ASSERT_NULL(heap_caps_malloc(SIZE_MAX-1, MALLOC_CAP_32BIT)); +#if !(CONFIG_ESP_SYSTEM_MEMPROT_FEATURE || CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT) TEST_ASSERT_NULL(heap_caps_malloc(SIZE_MAX-1, MALLOC_CAP_EXEC)); +#endif } TEST_CASE("unreasonable allocs should all fail", "[heap]") diff --git a/components/heap/test_apps/heap_tests/main/test_malloc_caps.c b/components/heap/test_apps/heap_tests/main/test_malloc_caps.c index 3b43b65cb0..f9e79791e8 100644 --- a/components/heap/test_apps/heap_tests/main/test_malloc_caps.c +++ b/components/heap/test_apps/heap_tests/main/test_malloc_caps.c @@ -336,13 +336,13 @@ TEST_CASE("RTC memory should be lowest priority and its free size should be big } #endif +#if !(CONFIG_ESP_SYSTEM_MEMPROT_FEATURE || CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT) TEST_CASE("test memory protection features", "[heap][mem_prot]") { // try to allocate memory in IRAM and check that if memory protection is active, // no memory is being allocated uint32_t *iram_ptr = heap_caps_malloc(4, MALLOC_CAP_EXEC); -#if !CONFIG_ESP_SYSTEM_MEMPROT // System memory protection not active, check that iram_ptr is not null // Check that iram_ptr is in IRAM TEST_ASSERT_NOT_NULL(iram_ptr); @@ -350,8 +350,5 @@ TEST_CASE("test memory protection features", "[heap][mem_prot]") // free the memory heap_caps_free(iram_ptr); -#else - // System memory protection is active, DIRAM seen as DRAM, iram_ptr should be null - TEST_ASSERT_NULL(iram_ptr); -#endif // !CONFIG_ESP_SYSTEM_MEMPROT } +#endif // !(CONFIG_ESP_SYSTEM_MEMPROT_FEATURE || CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT) diff --git a/docs/en/migration-guides/release-6.x/6.0/system.rst b/docs/en/migration-guides/release-6.x/6.0/system.rst index e0a4985819..f3094bb349 100644 --- a/docs/en/migration-guides/release-6.x/6.0/system.rst +++ b/docs/en/migration-guides/release-6.x/6.0/system.rst @@ -244,3 +244,10 @@ ULP --- The LP-Core will now wake up the main CPU when it encounters an exception during deep sleep. This feature is enabled by default but can be disabled via the :ref:`CONFIG_ULP_TRAP_WAKEUP` Kconfig option is this behavior is not desired. + +Heap +---- + +In previous versions of ESP-IDF, the capability MALLOC_CAP_EXEC would be available regardless of the memory protection configuration state. This implied that a call to e.g., :cpp:func:`heap_caps_malloc` with MALLOC_CAP_EXEC would return NULL when CONFIG_ESP_SYSTEM_MEMPROT_FEATURE or CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT are enabled. + +Since ESP-IDF v6.0, the definition of MALLOC_CAP_EXEC is conditional, meaning that if CONFIG_ESP_SYSTEM_MEMPROT is enabled, MALLOC_CAP_EXEC will not be defined. Therefore, using it will generate a compile time error. diff --git a/tools/idf_py_actions/hints.yml b/tools/idf_py_actions/hints.yml index ffc2d1887a..f9a265c46c 100644 --- a/tools/idf_py_actions/hints.yml +++ b/tools/idf_py_actions/hints.yml @@ -697,3 +697,7 @@ - re_variables: ['w5500'] hint_variables: ['W5500', 'w5500'] + +- + re: "error: 'MALLOC_CAP_EXEC' undeclared \\(first use in this function\\)" + hint: "MALLOC_CAP_EXEC capability cannot be used if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT or CONFIG_ESP_SYSTEM_MEMPROT_FEATURE is enlabled. \nFor further information about those configurations, run 'idf.py docs -sp api-reference/kconfig-reference.html#memory-protection'"