From 889cfabc8915c889e185f4324d28fc46e57393e7 Mon Sep 17 00:00:00 2001 From: Martin Vychodil Date: Wed, 19 Feb 2025 09:42:57 +0100 Subject: [PATCH] fix(security): Fixed ESP32S2 memory protection check for Peri1 RTCSLOW interrupt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - fixes the issue found in https://github.com/espressif/esp-idf/issues/15359 - extends debug printouts in the related tests --- .../hal/esp32s2/include/hal/memprot_peri_ll.h | 4 ++-- tools/ci/check_copyright_ignore.txt | 1 - .../memprot/main/esp32s2/test_memprot_main.c | 21 +++++++++++++++---- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/components/hal/esp32s2/include/hal/memprot_peri_ll.h b/components/hal/esp32s2/include/hal/memprot_peri_ll.h index f38bdee1f145..98f946f6f399 100644 --- a/components/hal/esp32s2/include/hal/memprot_peri_ll.h +++ b/components/hal/esp32s2/include/hal/memprot_peri_ll.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -109,7 +109,7 @@ static inline intptr_t memprot_ll_peri1_rtcslow_get_fault_address(void) static inline bool memprot_ll_peri1_rtcslow_is_intr_mine(void) { - if (memprot_ll_dram0_is_assoc_intr()) { + if (memprot_ll_peri1_is_assoc_intr()) { uint32_t faulting_address = (uint32_t)memprot_ll_peri1_rtcslow_get_fault_address(); return faulting_address >= PERI1_RTCSLOW_ADDRESS_LOW && faulting_address <= PERI1_RTCSLOW_ADDRESS_HIGH; } diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index 90e573deab34..eea8adac3f15 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -1859,7 +1859,6 @@ tools/test_apps/system/gdb_loadable_elf/main/hello_world_main.c tools/test_apps/system/longjmp_test/app_test.py tools/test_apps/system/longjmp_test/main/hello_world_main.c tools/test_apps/system/memprot/app_test.py -tools/test_apps/system/memprot/main/esp32s2/test_memprot_main.c tools/test_apps/system/monitor_ide_integration/app_test.py tools/test_apps/system/monitor_ide_integration/main/main.c tools/test_apps/system/no_embedded_paths/check_for_file_paths.py diff --git a/tools/test_apps/system/memprot/main/esp32s2/test_memprot_main.c b/tools/test_apps/system/memprot/main/esp32s2/test_memprot_main.c index b0a596d5d327..5b29113cf086 100644 --- a/tools/test_apps/system/memprot/main/esp32s2/test_memprot_main.c +++ b/tools/test_apps/system/memprot/main/esp32s2/test_memprot_main.c @@ -1,4 +1,9 @@ -/* MEMPROT IramDram testing code */ +/* + * SPDX-FileCopyrightText: 2020-2025 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + #include #include #include "sdkconfig.h" @@ -7,6 +12,8 @@ #include "esp32s2/memprot.h" #include "soc/soc.h" +static const char *TAG = "memprot_test_ESP32S2"; + /* * ESP32S2 MEMORY PROTECTION MODULE TEST * ===================================== @@ -108,7 +115,6 @@ static uint8_t RTC_SLOW_ATTR rtcslow_dummy_buffer[2 * SRAM_TEST_BUFFER_SIZE] = { * testing regions and splitting address scheme * */ - static uint32_t *test_memprot_dram0_rtcfast_get_min_split_addr(void) { return (uint32_t *)(rtcfast_dummy_buffer + sizeof(rtcfast_dummy_buffer) / 2); @@ -187,7 +193,6 @@ static uint32_t *test_memprot_addr_high(mem_type_prot_t mem_type) } } - static uint32_t *test_memprot_get_split_addr(mem_type_prot_t mem_type) { switch (mem_type) { @@ -210,7 +215,6 @@ static uint32_t *test_memprot_get_split_addr(mem_type_prot_t mem_type) } } - /* * testing setup of the memory-protection module */ @@ -356,9 +360,11 @@ static void test_memprot_read(mem_type_prot_t mem_type) bool write_perm_low, write_perm_high, read_perm_low, read_perm_high; esp_memprot_get_perm_write(mem_type, &write_perm_low, &write_perm_high); esp_memprot_get_perm_read(mem_type, &read_perm_low, &read_perm_high); + ESP_EARLY_LOGD(TAG, "TEST_READ (low: r=%u w=%u, high: r=%u w=%u):", read_perm_low, write_perm_low, read_perm_high, write_perm_high); volatile uint32_t *ptr_low = test_memprot_addr_low(mem_type); volatile uint32_t *ptr_high = test_memprot_addr_high(mem_type); + ESP_EARLY_LOGD(TAG, "[test_addr_low=0x%08X test_addr_high=0x%08X]", ptr_low, ptr_high); //temporarily allow WRITE for setting the test values esp_memprot_set_write_perm(mem_type, true, true); @@ -398,12 +404,14 @@ static void test_memprot_write(mem_type_prot_t mem_type) bool write_perm_low, write_perm_high, read_perm_low, read_perm_high; esp_memprot_get_perm_write(mem_type, &write_perm_low, &write_perm_high); esp_memprot_get_perm_read(mem_type, &read_perm_low, &read_perm_high); + ESP_EARLY_LOGD(TAG, "TEST_WRITE (low: r=%u w=%u, high: r=%u w=%u):", read_perm_low, write_perm_low, read_perm_high, write_perm_high); //temporarily allow READ operation esp_memprot_set_read_perm(mem_type, true, true); volatile uint32_t *ptr_low = test_memprot_addr_low(mem_type); volatile uint32_t *ptr_high = test_memprot_addr_high(mem_type); + ESP_EARLY_LOGD(TAG, "[test_addr_low=0x%08X test_addr_high=0x%08X]", ptr_low, ptr_high); //perform WRITE in low region const uint32_t test_val = 10; @@ -448,8 +456,13 @@ static void test_memprot_exec(mem_type_prot_t mem_type) bool exec_perm_low, exec_perm_high; esp_memprot_get_perm_exec(mem_type, &exec_perm_low, &exec_perm_high); + bool read_perm_low, read_perm_high; + esp_memprot_get_perm_read(mem_type, &read_perm_low, &read_perm_high); + ESP_EARLY_LOGD(TAG, "TEST_EXEC (low: r=%u w=%u x=%u, high: r=%u w=%u x=%u):", read_perm_low, write_perm_low, exec_perm_low, read_perm_high, write_perm_high, exec_perm_high); + volatile uint32_t *fnc_ptr_low = test_memprot_addr_low(mem_type); volatile uint32_t *fnc_ptr_high = test_memprot_addr_high(mem_type); + ESP_EARLY_LOGD(TAG, "[test_addr_low=0x%08X test_addr_high=0x%08X]", fnc_ptr_low, fnc_ptr_high); //enable WRITE permission for both segments esp_memprot_set_write_perm(mem_type, true, true);