Merge branch 'feat/make-use-of-exec-cap-compile-time-error' into 'master'

feat(heap): Make MALLOC_CAP_EXEC illegal use a compile time error

Closes IDFGH-14014 and IDF-11690

See merge request espressif/esp-idf!34903

Author: SoucheSouche

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

components/heap/Kconfig

@@ -1,5 +1,10 @@
 menu "Heap memory debugging"
 
+    config HEAP_HAS_EXEC_HEAP
+        bool
+        default n if ESP_SYSTEM_MEMPROT
+        default y if !ESP_SYSTEM_MEMPROT
+
     choice HEAP_CORRUPTION_DETECTION
         prompt "Heap corruption detection"
         default HEAP_POISONING_DISABLED

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_HEAP_HAS_EXEC_HEAP
+    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,

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_HEAP_HAS_EXEC_HEAP
 #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

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 }},
 };

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");

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_HEAP_HAS_EXEC_HEAP
 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_HEAP_HAS_EXEC_HEAP

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_HEAP_HAS_EXEC_HEAP
     TEST_ASSERT_NULL(heap_caps_malloc(SIZE_MAX-1, MALLOC_CAP_EXEC));
+#endif
 }
 
 TEST_CASE("unreasonable allocs should all fail", "[heap]")

components/heap/test_apps/heap_tests/main/test_malloc_caps.c

@@ -18,7 +18,7 @@
 #include <stdlib.h>
 #include <sys/param.h>
 
-#if !CONFIG_ESP_SYSTEM_MEMPROT && !CONFIG_HEAP_TASK_TRACKING
+#if CONFIG_HEAP_HAS_EXEC_HEAP && !(CONFIG_HEAP_TASK_TRACKING)
 TEST_CASE("Capabilities allocator test", "[heap]")
 {
     char *m1, *m2[10];
@@ -108,7 +108,7 @@ TEST_CASE("Capabilities allocator test", "[heap]")
     free(m1);
     printf("Done.\n");
 }
-#endif // !CONFIG_ESP_SYSTEM_MEMPROT && !CONFIG_HEAP_TASK_TRACKING
+#endif // CONFIG_HEAP_HAS_EXEC_HEAP && !(CONFIG_HEAP_TASK_TRACKING)
 
 #ifdef CONFIG_ESP32_IRAM_AS_8BIT_ACCESSIBLE_MEMORY
 TEST_CASE("IRAM_8BIT capability test", "[heap]")
@@ -230,7 +230,7 @@ TEST_CASE("heap caps minimum free bytes fault cases", "[heap]")
 /* Small function runs from IRAM to check that malloc/free/realloc
    all work OK when cache is disabled...
 */
-#if !CONFIG_ESP_SYSTEM_MEMPROT && !CONFIG_HEAP_PLACE_FUNCTION_INTO_FLASH && !CONFIG_HEAP_TASK_TRACKING
+#if CONFIG_HEAP_HAS_EXEC_HEAP && !CONFIG_HEAP_PLACE_FUNCTION_INTO_FLASH && !CONFIG_HEAP_TASK_TRACKING
 static IRAM_ATTR __attribute__((noinline)) bool iram_malloc_test(void)
 {
     spi_flash_guard_get()->start(); // Disables flash cache
@@ -252,7 +252,7 @@ TEST_CASE("heap_caps_xxx functions work with flash cache disabled", "[heap]")
 {
     TEST_ASSERT( iram_malloc_test() );
 }
-#endif // !CONFIG_ESP_SYSTEM_MEMPROT && !CONFIG_HEAP_PLACE_FUNCTION_INTO_FLASH && !CONFIG_HEAP_TASK_TRACKING
+#endif // CONFIG_HEAP_HAS_EXEC_HEAP && !CONFIG_HEAP_PLACE_FUNCTION_INTO_FLASH && !CONFIG_HEAP_TASK_TRACKING
 
 #ifdef CONFIG_HEAP_ABORT_WHEN_ALLOCATION_FAILS
 TEST_CASE("When enabled, allocation operation failure generates an abort", "[heap][reset=abort,SW_CPU_RESET]")
@@ -336,22 +336,19 @@ TEST_CASE("RTC memory should be lowest priority and its free size should be big
 }
 #endif
 
+#if CONFIG_HEAP_HAS_EXEC_HEAP
 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);
     TEST_ASSERT(true == esp_ptr_in_iram(iram_ptr));
 
     // 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_HEAP_HAS_EXEC_HEAP

docs/en/migration-guides/release-6.x/6.0/system.rst

@@ -246,3 +246,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.