Merge branch 'bugfix/esp32s3_memprot_wrong_check_unicore' into 'master'

System/Security: fix missing checks for CPU-count sensitive Memprot APIs (ESP32S3)

Closes IDF-5401

See merge request espressif/esp-idf!18834

components/esp_hw_support/include/soc/esp32s3/soc_memprot_types.h

@@ -78,27 +78,42 @@ typedef struct {
     int target_cpu[portNUM_PROCESSORS];  /*!< Array of CPU/core IDs required to receive given PMS protection */
 } esp_memp_config_t;
 
+//2-CPU configuration
 #if portNUM_PROCESSORS > 1
+
+//default IDF configuration (basic memory regions, split line detection, locked, panic mode on)
 #define ESP_MEMPROT_DEFAULT_CONFIG() { \
     .invoke_panic_handler = true, \
     .lock_feature = true, \
     .split_addr = NULL, \
     /* .mem_type_mask = MEMPROT_TYPE_ALL, \ - unless IDF-5208 gets merged */ \
     .mem_type_mask = MEMPROT_TYPE_IRAM0_SRAM | MEMPROT_TYPE_DRAM0_SRAM, \
-    .target_cpu_count = portNUM_PROCESSORS, \
+    .target_cpu_count = 2, \
     .target_cpu = {PRO_CPU_NUM, APP_CPU_NUM} \
 }
-#else
+//zero (no-go) configuration
+#define ESP_MEMPROT_ZERO_CONFIG() { \
+    .target_cpu_count = 2, \
+    .target_cpu = {PRO_CPU_NUM, APP_CPU_NUM} \
+}
+
+#else //1-CPU configuration
+
 #define ESP_MEMPROT_DEFAULT_CONFIG() { \
     .invoke_panic_handler = true, \
     .lock_feature = true, \
     .split_addr = NULL,                \
     /* .mem_type_mask = MEMPROT_TYPE_ALL, \ - unless IDF-5208 gets merged */ \
     .mem_type_mask = MEMPROT_TYPE_IRAM0_SRAM | MEMPROT_TYPE_DRAM0_SRAM, \
-    .target_cpu_count = portNUM_PROCESSORS, \
+    .target_cpu_count = 1, \
     .target_cpu = {PRO_CPU_NUM} \
 }
-#endif
+#define ESP_MEMPROT_ZERO_CONFIG() { \
+    .target_cpu_count = 1, \
+    .target_cpu = {PRO_CPU_NUM} \
+}
+
+#endif //end of CPU-count based defines
 
 /**
  * @brief Converts Memory protection type to string

components/esp_hw_support/port/esp32s3/esp_memprot.c

@@ -18,23 +18,28 @@
 #include "esp_private/esp_memprot_internal.h"
 #include "esp_memprot.h"
 
+/*
+ * LD section boundaries
+ */
 extern int _iram_text_start;
 extern int _iram_text_end;
 extern int _rtc_text_end;
 
+/*
+ * Local holder of the Memprot config required by the last esp_mprot_set_prot() call.
+ * The structure is zeroed on creation => 'mem_type_mask == MEMPROT_TYPE_NONE' guarantees no interference before proper
+ * update by the API function
+ */
+static esp_memp_config_t s_memp_cfg = ESP_MEMPROT_ZERO_CONFIG();
+
+
 //////////////////////////////////////////////////////////////////////////////
 // internal helpers
 
-#if portNUM_PROCESSORS > 1
-static const int s_chip_cpu[] = {PRO_CPU_NUM, APP_CPU_NUM};
-#else
-static const int s_chip_cpu[] = {PRO_CPU_NUM};
-#endif
-
 static esp_err_t esp_mprot_cpuid_valid(const int core)
 {
-    for (size_t x = 0; x < portNUM_PROCESSORS; x++) {
+    for (size_t x = 0; x < s_memp_cfg.target_cpu_count; x++) {
-        if (core == s_chip_cpu[x]) {
+        if (core == s_memp_cfg.target_cpu[x]) {
             return ESP_OK;
         }
     }
@@ -510,44 +515,68 @@ esp_err_t IRAM_ATTR esp_mprot_get_active_intr(esp_memp_intr_source_t *active_mem
         uint32_t intr_on = 0;
         esp_err_t err;
 
+        //IRAM0
+        if (s_memp_cfg.mem_type_mask & MEMPROT_TYPE_IRAM0_SRAM) {
+
             mt = MEMPROT_TYPE_IRAM0_SRAM;
+
             c = PRO_CPU_NUM;
             ESP_MEMPROT_ERR_CHECK(err, esp_mprot_ll_err_to_esp_err(memprot_ll_iram0_get_monitor_status_intr(c, &intr_on)))
             if (intr_on) {
                 break;
             }
 
+            //2-core
+            if (s_memp_cfg.target_cpu_count > 1) {
                 c = APP_CPU_NUM;
                 ESP_MEMPROT_ERR_CHECK(err, esp_mprot_ll_err_to_esp_err(memprot_ll_iram0_get_monitor_status_intr(c, &intr_on)))
                 if (intr_on) {
                     break;
                 }
+            }
+        }
+
+        //DRAM0
+        if (s_memp_cfg.mem_type_mask & MEMPROT_TYPE_DRAM0_SRAM) {
 
             mt = MEMPROT_TYPE_DRAM0_SRAM;
+
             c = PRO_CPU_NUM;
             ESP_MEMPROT_ERR_CHECK(err, esp_mprot_ll_err_to_esp_err(memprot_ll_dram0_get_monitor_status_intr(c, &intr_on)))
             if (intr_on) {
                 break;
             }
 
+            //2-core
+            if (s_memp_cfg.target_cpu_count > 1) {
                 c = APP_CPU_NUM;
                 ESP_MEMPROT_ERR_CHECK(err, esp_mprot_ll_err_to_esp_err(memprot_ll_dram0_get_monitor_status_intr(c, &intr_on)))
                 if (intr_on) {
                     break;
                 }
+            }
+        }
+
+        //RTCFAST
+        if (s_memp_cfg.mem_type_mask & MEMPROT_TYPE_IRAM0_RTCFAST) {
 
             mt = MEMPROT_TYPE_IRAM0_RTCFAST;
+
             c = PRO_CPU_NUM;
             ESP_MEMPROT_ERR_CHECK(err, esp_mprot_ll_err_to_esp_err(memprot_ll_rtcfast_get_monitor_status_intr(c, &intr_on)))
             if (intr_on) {
                 break;
             }
 
+            //2-core
+            if (s_memp_cfg.target_cpu_count > 1) {
                 c = APP_CPU_NUM;
                 ESP_MEMPROT_ERR_CHECK(err, esp_mprot_ll_err_to_esp_err(memprot_ll_rtcfast_get_monitor_status_intr(c, &intr_on)))
                 if (intr_on) {
                     break;
                 }
+            }
+        }
 
         mt = MEMPROT_TYPE_NONE;
         c = -1;
@@ -592,32 +621,64 @@ esp_err_t IRAM_ATTR esp_mprot_is_conf_locked_any(bool *locked)
     }
 
     bool lock_on = false;
-
     esp_err_t err;
+
+    //IRAM0
+    if (s_memp_cfg.mem_type_mask & MEMPROT_TYPE_IRAM0_SRAM) {
+
         ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_split_addr_lock(MEMPROT_TYPE_IRAM0_SRAM, &lock_on, DEFAULT_CPU_NUM))
         *locked |= lock_on;
-    ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_split_addr_lock(MEMPROT_TYPE_DRAM0_SRAM, &lock_on, DEFAULT_CPU_NUM))
+
-    *locked |= lock_on;
         ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_pms_lock(MEMPROT_TYPE_IRAM0_SRAM, &lock_on, DEFAULT_CPU_NUM))
         *locked |= lock_on;
-    ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_pms_lock(MEMPROT_TYPE_DRAM0_SRAM, &lock_on, DEFAULT_CPU_NUM))
+
-    *locked |= lock_on;
-    ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_pms_lock(MEMPROT_TYPE_IRAM0_RTCFAST, &lock_on, PRO_CPU_NUM))
-    *locked |= lock_on;
-    ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_pms_lock(MEMPROT_TYPE_IRAM0_RTCFAST, &lock_on, APP_CPU_NUM))
-    *locked |= lock_on;
         ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_lock(MEMPROT_TYPE_IRAM0_SRAM, &lock_on, PRO_CPU_NUM))
         *locked |= lock_on;
+
+        //2-core
+        if (s_memp_cfg.target_cpu_count > 1) {
             ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_lock(MEMPROT_TYPE_IRAM0_SRAM, &lock_on, APP_CPU_NUM))
             *locked |= lock_on;
+        }
+    }
+
+    //DRAM0
+    if (s_memp_cfg.mem_type_mask & MEMPROT_TYPE_DRAM0_SRAM) {
+
+        ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_split_addr_lock(MEMPROT_TYPE_DRAM0_SRAM, &lock_on, DEFAULT_CPU_NUM))
+        *locked |= lock_on;
+
+        ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_pms_lock(MEMPROT_TYPE_DRAM0_SRAM, &lock_on, DEFAULT_CPU_NUM))
+        *locked |= lock_on;
+
         ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_lock(MEMPROT_TYPE_DRAM0_SRAM, &lock_on, PRO_CPU_NUM));
         *locked |= lock_on;
+
+        //2-core
+        if (s_memp_cfg.target_cpu_count > 1) {
             ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_lock(MEMPROT_TYPE_DRAM0_SRAM, &lock_on, APP_CPU_NUM));
             *locked |= lock_on;
+        }
+    }
+
+    //RTCFAST
+    if (s_memp_cfg.mem_type_mask & MEMPROT_TYPE_IRAM0_RTCFAST) {
+
+        ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_pms_lock(MEMPROT_TYPE_IRAM0_RTCFAST, &lock_on, PRO_CPU_NUM))
+        *locked |= lock_on;
+
         ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_lock(MEMPROT_TYPE_IRAM0_RTCFAST, &lock_on, PRO_CPU_NUM));
         *locked |= lock_on;
+
+        //2-core
+        if (s_memp_cfg.target_cpu_count > 1) {
+            ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_pms_lock(MEMPROT_TYPE_IRAM0_RTCFAST, &lock_on, APP_CPU_NUM))
+            *locked |= lock_on;
+
             ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_lock(MEMPROT_TYPE_IRAM0_RTCFAST, &lock_on, APP_CPU_NUM));
             *locked |= lock_on;
+        }
+    }
 
     return ESP_OK;
 }
@@ -629,20 +690,40 @@ esp_err_t IRAM_ATTR esp_mprot_is_intr_ena_any(bool *enabled)
     }
 
     bool ena_on = false;
-
     esp_err_t err;
+
+    //IRAM0
+    if (s_memp_cfg.mem_type_mask & MEMPROT_TYPE_IRAM0_SRAM) {
         ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_en(MEMPROT_TYPE_IRAM0_SRAM, &ena_on, PRO_CPU_NUM))
         *enabled |= ena_on;
+        //2-core
+        if (s_memp_cfg.target_cpu_count > 1) {
             ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_en(MEMPROT_TYPE_IRAM0_SRAM, &ena_on, APP_CPU_NUM))
             *enabled |= ena_on;
+        }
+    }
+
+    //DRAM0
+    if (s_memp_cfg.mem_type_mask & MEMPROT_TYPE_DRAM0_SRAM) {
         ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_en(MEMPROT_TYPE_DRAM0_SRAM, &ena_on, PRO_CPU_NUM))
         *enabled |= ena_on;
+        //2-core
+        if (s_memp_cfg.target_cpu_count > 1) {
             ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_en(MEMPROT_TYPE_DRAM0_SRAM, &ena_on, APP_CPU_NUM))
             *enabled |= ena_on;
+        }
+    }
+
+    //RTCFAST
+    if (s_memp_cfg.mem_type_mask & MEMPROT_TYPE_IRAM0_RTCFAST) {
         ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_en(MEMPROT_TYPE_IRAM0_RTCFAST, &ena_on, PRO_CPU_NUM))
         *enabled |= ena_on;
+        //2-core
+        if (s_memp_cfg.target_cpu_count > 1) {
             ESP_MEMPROT_ERR_CHECK(err, esp_mprot_get_monitor_en(MEMPROT_TYPE_IRAM0_RTCFAST, &ena_on, APP_CPU_NUM))
             *enabled |= ena_on;
+        }
+    }
 
     return ESP_OK;
 }
@@ -1130,6 +1211,11 @@ esp_err_t esp_mprot_set_prot(const esp_memp_config_t *memp_config)
         }
     }
 
+    //keep current configuration copy if all went well
+    if (ret == ESP_OK) {
+        s_memp_cfg = *memp_config;
+    }
+
     return ret;
 }
 
@@ -1199,9 +1285,8 @@ esp_err_t esp_mprot_dump_configuration(char **dump_info_string)
     memprot_ll_dram0_get_pms_area_2(&ar2d, &aw2d);
     memprot_ll_dram0_get_pms_area_3(&ar3d, &aw3d);
 
-    bool rtc_line_lock_0, rtc_line_lock_1;
+    bool rtc_line_lock_0;
     memprot_ll_get_pif_constraint_lock(PRO_CPU_NUM, &rtc_line_lock_0);
-    memprot_ll_get_pif_constraint_lock(APP_CPU_NUM, &rtc_line_lock_1);
 
     sprintf((*dump_info_string + offset),
             "PMS area settings:\n"
@@ -1234,6 +1319,11 @@ esp_err_t esp_mprot_dump_configuration(char **dump_info_string)
 
     offset = strlen(*dump_info_string);
 
+    //2-CPU setup
+    if (s_memp_cfg.target_cpu_count > 1) {
+
+        bool rtc_line_lock_1;
+        memprot_ll_get_pif_constraint_lock(APP_CPU_NUM, &rtc_line_lock_1);
         sprintf((*dump_info_string + offset), " RTCFAST (APP_CPU, lock=%u):\n", rtc_line_lock_1);
 
         offset = strlen(*dump_info_string);
@@ -1255,6 +1345,7 @@ esp_err_t esp_mprot_dump_configuration(char **dump_info_string)
         }
 
         offset = strlen(*dump_info_string);
+    }
 
     return ESP_OK;
 }