From ff839be31d181516a6f8bb2fa3bafb708d0b38cf Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Wed, 17 Jan 2024 13:40:49 +0530 Subject: [PATCH 1/3] fix(esp_hw_support): Fix the I/DCACHE region PMP protection --- .../port/esp32c6/cpu_region_protect.c | 51 +++++++++++++++++-- .../port/esp32h2/cpu_region_protect.c | 51 +++++++++++++++++-- 2 files changed, 94 insertions(+), 8 deletions(-) diff --git a/components/esp_hw_support/port/esp32c6/cpu_region_protect.c b/components/esp_hw_support/port/esp32c6/cpu_region_protect.c index 9dcadc4173..054d7451ce 100644 --- a/components/esp_hw_support/port/esp32c6/cpu_region_protect.c +++ b/components/esp_hw_support/port/esp32c6/cpu_region_protect.c @@ -13,17 +13,45 @@ #ifdef BOOTLOADER_BUILD // Without L bit set #define CONDITIONAL_NONE 0x0 +#define CONDITIONAL_R PMP_R #define CONDITIONAL_RX PMP_R | PMP_X #define CONDITIONAL_RW PMP_R | PMP_W #define CONDITIONAL_RWX PMP_R | PMP_W | PMP_X #else // With L bit set #define CONDITIONAL_NONE NONE +#define CONDITIONAL_R R #define CONDITIONAL_RX RX #define CONDITIONAL_RW RW #define CONDITIONAL_RWX RWX #endif +#define ALIGN_UP_TO_MMU_PAGE_SIZE(addr) (((addr) + (SOC_MMU_PAGE_SIZE) - 1) & ~((SOC_MMU_PAGE_SIZE) - 1)) +#define ALIGN_DOWN_TO_MMU_PAGE_SIZE(addr) ((addr) & ~((SOC_MMU_PAGE_SIZE) - 1)) + +/** + * @brief Generate the PMP address field value for PMPCFG.A == NAPOT + * + * NOTE: Here, (end-start) must be a power of 2 size and start must + * be aligned to this size. This API returns UINT32_MAX on failing + * these conditions, which when plugged into the PMP entry registers + * does nothing. This skips the corresponding region's protection. + * + * @param start Region starting address + * @param end Region ending address + * + * @return uint32_t PMP address field value + */ +static inline uint32_t pmpaddr_napot(uint32_t start, uint32_t end) +{ + uint32_t size = end - start; + if ((size & (size - 1)) || (start % size)) { + return UINT32_MAX; + } + + return start | ((size - 1) >> 1); +} + static void esp_cpu_configure_invalid_regions(void) { const unsigned PMA_NONE = PMA_L | PMA_EN; @@ -155,15 +183,30 @@ void esp_cpu_configure_region_protection(void) #endif } +#if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD + extern int _instruction_reserved_end; + extern int _rodata_reserved_start; + extern int _rodata_reserved_end; + + const uint32_t irom_resv_end = ALIGN_UP_TO_MMU_PAGE_SIZE((uint32_t)(&_instruction_reserved_end)); + const uint32_t drom_resv_start = ALIGN_DOWN_TO_MMU_PAGE_SIZE((uint32_t)(&_rodata_reserved_start)); + const uint32_t drom_resv_end = ALIGN_UP_TO_MMU_PAGE_SIZE((uint32_t)(&_rodata_reserved_end)); + // 4. I_Cache (flash) - const uint32_t pmpaddr8 = PMPADDR_NAPOT(SOC_IROM_LOW, SOC_IROM_HIGH); + PMP_ENTRY_CFG_RESET(8); + const uint32_t pmpaddr8 = pmpaddr_napot(SOC_IROM_LOW, irom_resv_end); PMP_ENTRY_SET(8, pmpaddr8, PMP_NAPOT | RX); - _Static_assert(SOC_IROM_LOW < SOC_IROM_HIGH, "Invalid I_Cache region"); // 5. D_Cache (flash) - const uint32_t pmpaddr9 = PMPADDR_NAPOT(SOC_DROM_LOW, SOC_DROM_HIGH); + PMP_ENTRY_CFG_RESET(9); + const uint32_t pmpaddr9 = pmpaddr_napot(drom_resv_start, drom_resv_end); PMP_ENTRY_SET(9, pmpaddr9, PMP_NAPOT | R); - _Static_assert(SOC_DROM_LOW < SOC_DROM_HIGH, "Invalid D_Cache region"); +#else + // 4. I_Cache / D_Cache (flash) + const uint32_t pmpaddr8 = PMPADDR_NAPOT(SOC_IROM_LOW, SOC_IROM_HIGH); + PMP_ENTRY_SET(8, pmpaddr8, PMP_NAPOT | CONDITIONAL_RX); + _Static_assert(SOC_IROM_LOW < SOC_IROM_HIGH, "Invalid I/D_Cache region"); +#endif // 6. LP memory #if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD diff --git a/components/esp_hw_support/port/esp32h2/cpu_region_protect.c b/components/esp_hw_support/port/esp32h2/cpu_region_protect.c index 3eb7d03aad..a1073b688a 100644 --- a/components/esp_hw_support/port/esp32h2/cpu_region_protect.c +++ b/components/esp_hw_support/port/esp32h2/cpu_region_protect.c @@ -13,17 +13,45 @@ #ifdef BOOTLOADER_BUILD // Without L bit set #define CONDITIONAL_NONE 0x0 +#define CONDITIONAL_R PMP_R #define CONDITIONAL_RX PMP_R | PMP_X #define CONDITIONAL_RW PMP_R | PMP_W #define CONDITIONAL_RWX PMP_R | PMP_W | PMP_X #else // With L bit set #define CONDITIONAL_NONE NONE +#define CONDITIONAL_R R #define CONDITIONAL_RX RX #define CONDITIONAL_RW RW #define CONDITIONAL_RWX RWX #endif +#define ALIGN_UP_TO_MMU_PAGE_SIZE(addr) (((addr) + (SOC_MMU_PAGE_SIZE) - 1) & ~((SOC_MMU_PAGE_SIZE) - 1)) +#define ALIGN_DOWN_TO_MMU_PAGE_SIZE(addr) ((addr) & ~((SOC_MMU_PAGE_SIZE) - 1)) + +/** + * @brief Generate the PMP address field value for PMPCFG.A == NAPOT + * + * NOTE: Here, (end-start) must be a power of 2 size and start must + * be aligned to this size. This API returns UINT32_MAX on failing + * these conditions, which when plugged into the PMP entry registers + * does nothing. This skips the corresponding region's protection. + * + * @param start Region starting address + * @param end Region ending address + * + * @return uint32_t PMP address field value + */ +static inline uint32_t pmpaddr_napot(uint32_t start, uint32_t end) +{ + uint32_t size = end - start; + if ((size & (size - 1)) || (start % size)) { + return UINT32_MAX; + } + + return start | ((size - 1) >> 1); +} + static void esp_cpu_configure_invalid_regions(void) { const unsigned PMA_NONE = PMA_L | PMA_EN; @@ -155,15 +183,30 @@ void esp_cpu_configure_region_protection(void) #endif } +#if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD + extern int _instruction_reserved_end; + extern int _rodata_reserved_start; + extern int _rodata_reserved_end; + + const uint32_t irom_resv_end = ALIGN_UP_TO_MMU_PAGE_SIZE((uint32_t)(&_instruction_reserved_end)); + const uint32_t drom_resv_start = ALIGN_DOWN_TO_MMU_PAGE_SIZE((uint32_t)(&_rodata_reserved_start)); + const uint32_t drom_resv_end = ALIGN_UP_TO_MMU_PAGE_SIZE((uint32_t)(&_rodata_reserved_end)); + // 4. I_Cache (flash) - const uint32_t pmpaddr8 = PMPADDR_NAPOT(SOC_IROM_LOW, SOC_IROM_HIGH); + PMP_ENTRY_CFG_RESET(8); + const uint32_t pmpaddr8 = pmpaddr_napot(SOC_IROM_LOW, irom_resv_end); PMP_ENTRY_SET(8, pmpaddr8, PMP_NAPOT | RX); - _Static_assert(SOC_IROM_LOW < SOC_IROM_HIGH, "Invalid I_Cache region"); // 5. D_Cache (flash) - const uint32_t pmpaddr9 = PMPADDR_NAPOT(SOC_DROM_LOW, SOC_DROM_HIGH); + PMP_ENTRY_CFG_RESET(9); + const uint32_t pmpaddr9 = pmpaddr_napot(drom_resv_start, drom_resv_end); PMP_ENTRY_SET(9, pmpaddr9, PMP_NAPOT | R); - _Static_assert(SOC_DROM_LOW < SOC_DROM_HIGH, "Invalid D_Cache region"); +#else + // 4. I_Cache / D_Cache (flash) + const uint32_t pmpaddr8 = PMPADDR_NAPOT(SOC_IROM_LOW, SOC_IROM_HIGH); + PMP_ENTRY_SET(8, pmpaddr8, PMP_NAPOT | CONDITIONAL_RX); + _Static_assert(SOC_IROM_LOW < SOC_IROM_HIGH, "Invalid I/D_Cache region"); +#endif // 6. LP memory #if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT && !BOOTLOADER_BUILD From 366e4ee9449313218d10d350a446bd59e64089cb Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Thu, 22 Feb 2024 13:55:32 +0530 Subject: [PATCH 2/3] refactor(esp_hw_support): Remove redundant PMP entry for ROM region - The ROM text and data sections share the address range (see SOC_I/DROM_MASK_LOW - SOC_I/DROM_MASK_HIGH). - Initially, we had two PMP entries for this address range - one marking the region as RX and the other as R. - However, the latter entry is redundant as the former locks the PMP settings. - We can divide the ROM region into text and data sections later when we define boundaries marking these regions from the ROM. --- .../esp_hw_support/port/esp32c6/cpu_region_protect.c | 11 +++-------- .../esp_hw_support/port/esp32h2/cpu_region_protect.c | 11 +++-------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/components/esp_hw_support/port/esp32c6/cpu_region_protect.c b/components/esp_hw_support/port/esp32c6/cpu_region_protect.c index 054d7451ce..a33a9c34aa 100644 --- a/components/esp_hw_support/port/esp32c6/cpu_region_protect.c +++ b/components/esp_hw_support/port/esp32c6/cpu_region_protect.c @@ -125,7 +125,7 @@ void esp_cpu_configure_region_protection(void) * We also lock these entries so the R/W/X permissions are enforced even for machine mode */ const unsigned NONE = PMP_L; - const unsigned R = PMP_L | PMP_R; + __attribute__((unused)) const unsigned R = PMP_L | PMP_R; const unsigned RW = PMP_L | PMP_R | PMP_W; const unsigned RX = PMP_L | PMP_R | PMP_X; const unsigned RWX = PMP_L | PMP_R | PMP_W | PMP_X; @@ -144,15 +144,10 @@ void esp_cpu_configure_region_protection(void) PMP_ENTRY_SET(0, pmpaddr0, PMP_NAPOT | RWX); _Static_assert(SOC_CPU_SUBSYSTEM_LOW < SOC_CPU_SUBSYSTEM_HIGH, "Invalid CPU subsystem region"); - // 2.1 I-ROM + // 2.1 I/D-ROM PMP_ENTRY_SET(1, SOC_IROM_MASK_LOW, NONE); PMP_ENTRY_SET(2, SOC_IROM_MASK_HIGH, PMP_TOR | RX); - _Static_assert(SOC_IROM_MASK_LOW < SOC_IROM_MASK_HIGH, "Invalid I-ROM region"); - - // 2.2 D-ROM - PMP_ENTRY_SET(3, SOC_DROM_MASK_LOW, NONE); - PMP_ENTRY_SET(4, SOC_DROM_MASK_HIGH, PMP_TOR | R); - _Static_assert(SOC_DROM_MASK_LOW < SOC_DROM_MASK_HIGH, "Invalid D-ROM region"); + _Static_assert(SOC_IROM_MASK_LOW < SOC_IROM_MASK_HIGH, "Invalid I/D-ROM region"); if (esp_cpu_dbgr_is_attached()) { // Anti-FI check that cpu is really in ocd mode diff --git a/components/esp_hw_support/port/esp32h2/cpu_region_protect.c b/components/esp_hw_support/port/esp32h2/cpu_region_protect.c index a1073b688a..45617929b3 100644 --- a/components/esp_hw_support/port/esp32h2/cpu_region_protect.c +++ b/components/esp_hw_support/port/esp32h2/cpu_region_protect.c @@ -125,7 +125,7 @@ void esp_cpu_configure_region_protection(void) * We also lock these entries so the R/W/X permissions are enforced even for machine mode */ const unsigned NONE = PMP_L; - const unsigned R = PMP_L | PMP_R; + __attribute__((unused)) const unsigned R = PMP_L | PMP_R; const unsigned RW = PMP_L | PMP_R | PMP_W; const unsigned RX = PMP_L | PMP_R | PMP_X; const unsigned RWX = PMP_L | PMP_R | PMP_W | PMP_X; @@ -144,15 +144,10 @@ void esp_cpu_configure_region_protection(void) PMP_ENTRY_SET(0, pmpaddr0, PMP_NAPOT | RWX); _Static_assert(SOC_CPU_SUBSYSTEM_LOW < SOC_CPU_SUBSYSTEM_HIGH, "Invalid CPU subsystem region"); - // 2.1 I-ROM + // 2.1 I/D-ROM PMP_ENTRY_SET(1, SOC_IROM_MASK_LOW, NONE); PMP_ENTRY_SET(2, SOC_IROM_MASK_HIGH, PMP_TOR | RX); - _Static_assert(SOC_IROM_MASK_LOW < SOC_IROM_MASK_HIGH, "Invalid I-ROM region"); - - // 2.2 D-ROM - PMP_ENTRY_SET(3, SOC_DROM_MASK_LOW, NONE); - PMP_ENTRY_SET(4, SOC_DROM_MASK_HIGH, PMP_TOR | R); - _Static_assert(SOC_DROM_MASK_LOW < SOC_DROM_MASK_HIGH, "Invalid D-ROM region"); + _Static_assert(SOC_IROM_MASK_LOW < SOC_IROM_MASK_HIGH, "Invalid I/D-ROM region"); if (esp_cpu_dbgr_is_attached()) { // Anti-FI check that cpu is really in ocd mode From 2265c0f230fb1a15b4b0c0eaa6fcd7329052f335 Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Mon, 5 Feb 2024 17:18:44 +0530 Subject: [PATCH 3/3] feat(tools/test_apps): Add violation tests for the flash I/DROM region - For SoCs supporting PMP --- .../system/panic/main/include/test_memprot.h | 7 +++- .../system/panic/main/test_app_main.c | 8 +++- .../system/panic/main/test_memprot.c | 37 +++++++++++++++- tools/test_apps/system/panic/pytest_panic.py | 42 +++++++++++++++++-- 4 files changed, 87 insertions(+), 7 deletions(-) diff --git a/tools/test_apps/system/panic/main/include/test_memprot.h b/tools/test_apps/system/panic/main/include/test_memprot.h index c4b229a8b9..d21d795f60 100644 --- a/tools/test_apps/system/panic/main/include/test_memprot.h +++ b/tools/test_apps/system/panic/main/include/test_memprot.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -42,6 +42,11 @@ void test_rtc_slow_reg1_execute_violation(void); void test_rtc_slow_reg2_execute_violation(void); +void test_irom_reg_write_violation(void); + +void test_drom_reg_write_violation(void); + +void test_drom_reg_execute_violation(void); #ifdef __cplusplus } diff --git a/tools/test_apps/system/panic/main/test_app_main.c b/tools/test_apps/system/panic/main/test_app_main.c index 1d1f0c1cdc..aecd9f0836 100644 --- a/tools/test_apps/system/panic/main/test_app_main.c +++ b/tools/test_apps/system/panic/main/test_app_main.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -137,6 +137,12 @@ void app_main(void) HANDLE_TEST(test_name, test_rtc_slow_reg2_execute_violation); #endif +#if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT + HANDLE_TEST(test_name, test_irom_reg_write_violation); + HANDLE_TEST(test_name, test_drom_reg_write_violation); + HANDLE_TEST(test_name, test_drom_reg_execute_violation); +#endif + #endif die("Unknown test name"); diff --git a/tools/test_apps/system/panic/main/test_memprot.c b/tools/test_apps/system/panic/main/test_memprot.c index 3c559f1fc1..cfefa415e6 100644 --- a/tools/test_apps/system/panic/main/test_memprot.c +++ b/tools/test_apps/system/panic/main/test_memprot.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -15,7 +15,7 @@ #include "soc/soc.h" #include "test_memprot.h" - +#include "sdkconfig.h" #define RND_VAL (0xA5A5A5A5) #define SPIN_ITER (16) @@ -213,3 +213,36 @@ static void __attribute__((constructor)) test_print_rtc_var_func(void) printf("foo_s: %p | var_s: %p\n", &foo_s, &var_s); #endif } + + +/* ---------------------------------------------------- I/D Cache (Flash) Violation Checks ---------------------------------------------------- */ + +#if CONFIG_ESP_SYSTEM_PMP_IDRAM_SPLIT +static const uint16_t foo_buf[8] = { + 0x0001, 0x0001, 0x0001, 0x0001, + 0x0001, 0x0001, 0x0001, 0x0001, +}; + +void test_irom_reg_write_violation(void) +{ + extern int _instruction_reserved_end; + uint32_t *test_addr = (uint32_t *)((uint32_t)(&_instruction_reserved_end - 0x100)); + printf("Flash (IROM): Write operation | Address: %p\n", test_addr); + *test_addr = RND_VAL; +} + +void test_drom_reg_write_violation(void) +{ + uint32_t *test_addr = (uint32_t *)((uint32_t)(foo_buf)); + printf("Flash (DROM): Write operation | Address: %p\n", test_addr); + *test_addr = RND_VAL; +} + +void test_drom_reg_execute_violation(void) +{ + printf("Flash (DROM): Execute operation | Address: %p\n", foo_buf); + void (*func_ptr)(void); + func_ptr = (void(*)(void))foo_buf; + func_ptr(); +} +#endif diff --git a/tools/test_apps/system/panic/pytest_panic.py b/tools/test_apps/system/panic/pytest_panic.py index d96b20f200..08da655338 100644 --- a/tools/test_apps/system/panic/pytest_panic.py +++ b/tools/test_apps/system/panic/pytest_panic.py @@ -1,8 +1,9 @@ -# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: CC0-1.0 - import re -from typing import List, Optional, Union +from typing import List +from typing import Optional +from typing import Union import pexpect import pytest @@ -544,6 +545,11 @@ CONFIGS_MEMPROT_RTC_SLOW_MEM = [ pytest.param('memprot_esp32s2', marks=[pytest.mark.esp32s2]), ] +CONFIGS_MEMPROT_FLASH_IDROM = [ + pytest.param('memprot_esp32c6', marks=[pytest.mark.esp32c6]), + pytest.param('memprot_esp32h2', marks=[pytest.mark.esp32h2]) +] + @pytest.mark.parametrize('config', CONFIGS_MEMPROT_DCACHE, indirect=True) @pytest.mark.generic @@ -780,6 +786,36 @@ def test_rtc_slow_reg2_execute_violation(dut: PanicTestDut, test_func_name: str) dut.expect_cpu_reset() +@pytest.mark.parametrize('config', CONFIGS_MEMPROT_FLASH_IDROM, indirect=True) +@pytest.mark.generic +def test_irom_reg_write_violation(dut: PanicTestDut, test_func_name: str) -> None: + dut.run_test_func(test_func_name) + if dut.target == 'esp32c6': + dut.expect_gme('Store access fault') + elif dut.target == 'esp32h2': + dut.expect_gme('Cache error') + dut.expect_reg_dump(0) + dut.expect_cpu_reset() + + +@pytest.mark.parametrize('config', CONFIGS_MEMPROT_FLASH_IDROM, indirect=True) +@pytest.mark.generic +def test_drom_reg_write_violation(dut: PanicTestDut, test_func_name: str) -> None: + dut.run_test_func(test_func_name) + dut.expect_gme('Store access fault') + dut.expect_reg_dump(0) + dut.expect_cpu_reset() + + +@pytest.mark.parametrize('config', CONFIGS_MEMPROT_FLASH_IDROM, indirect=True) +@pytest.mark.generic +def test_drom_reg_execute_violation(dut: PanicTestDut, test_func_name: str) -> None: + dut.run_test_func(test_func_name) + dut.expect_gme('Instruction access fault') + dut.expect_reg_dump(0) + dut.expect_cpu_reset() + + @pytest.mark.esp32 @pytest.mark.parametrize('config', ['gdbstub_coredump'], indirect=True) def test_gdbstub_coredump(dut: PanicTestDut) -> None: