From 2a0e25ab74e5aba5e16c18ae5ba702ee1e74a16d Mon Sep 17 00:00:00 2001 From: "nilesh.kale" Date: Mon, 11 May 2026 09:50:38 +0530 Subject: [PATCH] fix(cpu_region_protect): set DROM mask PMP entry to read-only PMP entry 3 (SOC_DROM_MASK_HIGH, TOR mode) in the memprot path was incorrectly granted RW permission on esp32h21 and esp32c61. The mask ROM data region is inherently read-only; remove the W bit. Also added necessary tests to check voilations and re-enabled tests for ESP32P4 --- .../port/esp32c61/cpu_region_protect.c | 2 +- .../port/esp32h21/cpu_region_protect.c | 26 +++++++++++++---- .../system/panic/main/include/test_memprot.h | 8 +++++- .../system/panic/main/test_app_main.c | 4 +++ .../system/panic/main/test_memprot.c | 19 ++++++++++++- tools/test_apps/system/panic/pytest_panic.py | 28 +++++++++++++++++++ 6 files changed, 79 insertions(+), 8 deletions(-) diff --git a/components/esp_hw_support/port/esp32c61/cpu_region_protect.c b/components/esp_hw_support/port/esp32c61/cpu_region_protect.c index b44c638cbf6..cc62bade13b 100644 --- a/components/esp_hw_support/port/esp32c61/cpu_region_protect.c +++ b/components/esp_hw_support/port/esp32c61/cpu_region_protect.c @@ -147,7 +147,7 @@ void esp_cpu_configure_region_protection(void) if ((drom_start & (SOC_CPU_PMP_REGION_GRANULARITY - 1)) == 0) { PMP_ENTRY_SET(1, SOC_IROM_MASK_LOW, NONE); PMP_ENTRY_SET(2, drom_start, PMP_TOR | RX); - PMP_ENTRY_SET(3, SOC_DROM_MASK_HIGH, PMP_TOR | RW); + PMP_ENTRY_SET(3, SOC_DROM_MASK_HIGH, PMP_TOR | R); } else #endif { diff --git a/components/esp_hw_support/port/esp32h21/cpu_region_protect.c b/components/esp_hw_support/port/esp32h21/cpu_region_protect.c index 50ed436c771..bf397a80f5d 100644 --- a/components/esp_hw_support/port/esp32h21/cpu_region_protect.c +++ b/components/esp_hw_support/port/esp32h21/cpu_region_protect.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -104,7 +104,7 @@ void esp_cpu_configure_region_protection(void) const unsigned NONE = PMP_L; __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; + __attribute__((unused)) const unsigned RX = PMP_L | PMP_R | PMP_X; const unsigned RWX = PMP_L | PMP_R | PMP_W | PMP_X; // @@ -122,9 +122,25 @@ void esp_cpu_configure_region_protection(void) _Static_assert(SOC_CPU_SUBSYSTEM_LOW < SOC_CPU_SUBSYSTEM_HIGH, "Invalid CPU subsystem region"); // 2. I/D-ROM - const uint32_t pmpaddr1 = PMPADDR_NAPOT(SOC_IROM_MASK_LOW, SOC_IROM_MASK_HIGH); - PMP_ENTRY_SET(1, pmpaddr1, PMP_NAPOT | RX); - _Static_assert(SOC_IROM_MASK_LOW < SOC_IROM_MASK_HIGH, "Invalid I/D-ROM region"); + /* For non-MP (Mass Production) revisions of the chip, the PMP (Physical Memory Protection) entries 2 and 3 are not utilized for ROM memory mapping. + * In such cases, these entries remain unconfigured and vacant. + */ +#if CONFIG_ESP32H21_SELECTS_REV_MP && CONFIG_ESP_SYSTEM_MEMPROT && CONFIG_ESP_SYSTEM_MEMPROT_PMP && !BOOTLOADER_BUILD + const uint32_t drom_start = (uint32_t) (ets_rom_layout_p->drom_start); + if ((drom_start & (SOC_CPU_PMP_REGION_GRANULARITY - 1)) == 0) { + PMP_ENTRY_CFG_RESET(1); + PMP_ENTRY_CFG_RESET(2); + PMP_ENTRY_CFG_RESET(3); + PMP_ENTRY_SET(1, SOC_IROM_MASK_LOW, NONE); + PMP_ENTRY_SET(2, drom_start, PMP_TOR | RX); + PMP_ENTRY_SET(3, SOC_DROM_MASK_HIGH, PMP_TOR | R); + } else +#endif + { + const uint32_t pmpaddr1 = PMPADDR_NAPOT(SOC_IROM_MASK_LOW, SOC_IROM_MASK_HIGH); + PMP_ENTRY_SET(1, pmpaddr1, PMP_NAPOT | CONDITIONAL_RX); + _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/tools/test_apps/system/panic/main/include/test_memprot.h b/tools/test_apps/system/panic/main/include/test_memprot.h index e3e0dc40af6..7174d9c7505 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-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -63,6 +63,12 @@ void test_spiram_xip_irom_alignment_reg_execute_violation(void); void test_spiram_xip_drom_alignment_reg_execute_violation(void); +void test_irom_mask_reg_write_violation(void); + +#ifdef SOC_DROM_MASK_HIGH +void test_drom_mask_reg_write_violation(void); +#endif + void test_drom_reg_write_violation(void); void test_drom_reg_execute_violation(void); 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 8413507b16b..967c97d691d 100644 --- a/tools/test_apps/system/panic/main/test_app_main.c +++ b/tools/test_apps/system/panic/main/test_app_main.c @@ -188,6 +188,10 @@ void app_main(void) #if CONFIG_ESP_SYSTEM_MEMPROT HANDLE_TEST(test_name, test_irom_reg_write_violation); + HANDLE_TEST(test_name, test_irom_mask_reg_write_violation); +#ifdef SOC_DROM_MASK_HIGH + HANDLE_TEST(test_name, test_drom_mask_reg_write_violation); +#endif HANDLE_TEST(test_name, test_drom_reg_write_violation); HANDLE_TEST(test_name, test_drom_reg_execute_violation); diff --git a/tools/test_apps/system/panic/main/test_memprot.c b/tools/test_apps/system/panic/main/test_memprot.c index 209809127a2..420225405d1 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-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -299,6 +299,23 @@ void test_irom_reg_write_violation(void) *test_addr = RND_VAL; } +void test_irom_mask_reg_write_violation(void) +{ + uint32_t *test_addr = (uint32_t *)(SOC_IROM_MASK_LOW + 0x04); + printf("ROM (IROM Mask): Write operation | Address: %p\n", test_addr); + *test_addr = RND_VAL; +} + +#ifdef SOC_DROM_MASK_HIGH +void test_drom_mask_reg_write_violation(void) +{ + uint32_t *test_addr = (uint32_t *)(SOC_DROM_MASK_HIGH - 0x04); + printf("ROM (DROM Mask): Write operation | Address: %p\n", test_addr); + *test_addr = RND_VAL; +} + +#endif + void test_drom_reg_write_violation(void) { uint32_t *test_addr = (uint32_t *)((uint32_t)(foo_buf)); diff --git a/tools/test_apps/system/panic/pytest_panic.py b/tools/test_apps/system/panic/pytest_panic.py index 75b38143a21..f73d0f813a8 100644 --- a/tools/test_apps/system/panic/pytest_panic.py +++ b/tools/test_apps/system/panic/pytest_panic.py @@ -1103,6 +1103,34 @@ def test_non_cache_irom_reg_write_violation(dut: PanicTestDut, test_func_name: s irom_reg_write_violation(dut, test_func_name) +def irom_mask_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.generic +@pytest.mark.temp_skip_ci(targets=['esp32h21'], reason='lack of runners') +@idf_parametrize('config, target', CONFIGS_MEMPROT_FLASH_IDROM, indirect=['config', 'target']) +def test_irom_mask_reg_write_violation(dut: PanicTestDut, test_func_name: str) -> None: + irom_mask_reg_write_violation(dut, test_func_name) + + +def drom_mask_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.generic +@pytest.mark.temp_skip_ci(targets=['esp32h21'], reason='lack of runners') +@idf_parametrize('config, target', CONFIGS_MEMPROT_FLASH_IDROM, indirect=['config', 'target']) +def test_drom_mask_reg_write_violation(dut: PanicTestDut, test_func_name: str) -> None: + drom_mask_reg_write_violation(dut, test_func_name) + + def drom_reg_write_violation(dut: PanicTestDut, test_func_name: str) -> None: dut.run_test_func(test_func_name) dut.expect_gme('Store access fault')