Merge branch 'fix/esp_tee_flash_op_bound_checks' into 'master'

fix(esp_tee): Correct flash operation bound checks to handle all overlap cases

Closes IDF-14129

See merge request espressif/esp-idf!41946

components/bootloader_support/bootloader_flash/src/bootloader_flash.c

@@ -138,7 +138,7 @@ esp_err_t bootloader_flash_erase_range(uint32_t start_addr, uint32_t size)
 #include "esp_flash_partitions.h"
 #include "rom/spi_flash.h"
 
-extern bool esp_tee_flash_check_paddr_in_active_tee_part(size_t paddr);
+extern bool esp_tee_flash_check_prange_in_active_tee_part(const size_t paddr, const size_t len);
 #endif
 
 ESP_LOG_ATTR_TAG(TAG, "bootloader_flash");
@@ -524,7 +524,7 @@ esp_err_t bootloader_flash_write(size_t dest_addr, void *src, size_t size, bool
      * by validating the address before proceeding.
      */
 #if ESP_TEE_BUILD
-    bool addr_chk = esp_tee_flash_check_paddr_in_active_tee_part(dest_addr);
+    bool addr_chk = esp_tee_flash_check_prange_in_active_tee_part(dest_addr, size);
     if (addr_chk) {
         ESP_EARLY_LOGE(TAG, "bootloader_flash_write invalid dest_addr");
         return ESP_FAIL;
@@ -578,7 +578,7 @@ esp_err_t bootloader_flash_erase_sector(size_t sector)
 esp_err_t bootloader_flash_erase_range(uint32_t start_addr, uint32_t size)
 {
 #if ESP_TEE_BUILD
-    bool addr_chk = esp_tee_flash_check_paddr_in_active_tee_part(start_addr);
+    bool addr_chk = esp_tee_flash_check_prange_in_active_tee_part(start_addr, size);
     if (addr_chk) {
         return ESP_ERR_INVALID_ARG;
     }

components/esp_tee/scripts/esp32c5/sec_srv_tbl_default.yml

@@ -47,10 +47,7 @@ secure_services:
         type: IDF
         function: spi_flash_hal_erase_block
         args: 2
-      - id: 10
-        type: IDF
-        function: spi_flash_hal_erase_chip
-        args: 1
+      # ID: 10 empty
       - id: 11
         type: IDF
         function: spi_flash_hal_erase_sector

components/esp_tee/scripts/esp32c6/sec_srv_tbl_default.yml

@@ -43,10 +43,7 @@ secure_services:
         type: IDF
         function: spi_flash_hal_erase_block
         args: 2
-      - id: 9
-        type: IDF
-        function: spi_flash_hal_erase_chip
-        args: 1
+      # ID: 9 empty
       - id: 10
         type: IDF
         function: spi_flash_hal_erase_sector

components/esp_tee/scripts/esp32h2/sec_srv_tbl_default.yml

@@ -43,10 +43,7 @@ secure_services:
         type: IDF
         function: spi_flash_hal_erase_block
         args: 2
-      - id: 9
-        type: IDF
-        function: spi_flash_hal_erase_chip
-        args: 1
+      # ID: 9 empty
       - id: 10
         type: IDF
         function: spi_flash_hal_erase_sector

components/esp_tee/src/esp_secure_service_wrapper.c

@@ -422,11 +422,6 @@ void IRAM_ATTR __wrap_spi_flash_hal_erase_block(spi_flash_host_inst_t *host, uin
     esp_tee_service_call(3, SS_SPI_FLASH_HAL_ERASE_BLOCK, host, start_address);
 }
 
-void IRAM_ATTR __wrap_spi_flash_hal_erase_chip(spi_flash_host_inst_t *host)
-{
-    esp_tee_service_call(2, SS_SPI_FLASH_HAL_ERASE_CHIP, host);
-}
-
 void IRAM_ATTR __wrap_spi_flash_hal_erase_sector(spi_flash_host_inst_t *host, uint32_t start_address)
 {
     esp_tee_service_call(3, SS_SPI_FLASH_HAL_ERASE_SECTOR, host, start_address);

components/esp_tee/subproject/components/tee_flash_mgr/esp_tee_flash.c

@@ -208,3 +208,36 @@ bool esp_tee_flash_check_paddr_in_active_tee_part(const size_t paddr)
 {
     return ((paddr >= tee_prot_ctx.active_part_start_paddr) && (paddr < tee_prot_ctx.active_part_end_paddr));
 }
+
+bool esp_tee_flash_check_vrange_in_tee_region(const size_t vaddr, const size_t len)
+{
+    size_t vaddr_end = vaddr + len;
+    if (vaddr_end < vaddr) {
+        return true;
+    }
+
+    bool prot_reg1_overlap = ((vaddr < SOC_S_IROM_HIGH) && (vaddr_end > SOC_S_DROM_LOW));
+    bool prot_reg2_overlap = ((vaddr < SOC_MMU_END_VADDR) && (vaddr_end > SOC_S_MMU_MMAP_RESV_START_VADDR));
+
+    return (prot_reg1_overlap || prot_reg2_overlap);
+}
+
+bool esp_tee_flash_check_prange_in_tee_region(const size_t paddr, const size_t len)
+{
+    size_t paddr_end = paddr + len;
+    if (paddr_end < paddr) {
+        return true;
+    }
+
+    return ((paddr < tee_prot_ctx.flash_reg_end_paddr) && (paddr_end > tee_prot_ctx.flash_reg_start_paddr));
+}
+
+bool esp_tee_flash_check_prange_in_active_tee_part(const size_t paddr, const size_t len)
+{
+    size_t paddr_end = paddr + len;
+    if (paddr_end < paddr) {
+        return true;
+    }
+
+    return ((paddr < tee_prot_ctx.active_part_end_paddr) && (paddr_end > tee_prot_ctx.active_part_start_paddr));
+}

components/esp_tee/subproject/components/tee_flash_mgr/include/esp_tee_flash.h

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -80,7 +80,7 @@ esp_partition_info_t *esp_tee_flash_get_running_ree_partition(void);
 /**
  * @brief Check if the given virtual address falls within the TEE flash protected region
  *
- * @param addr Virtual address to check
+ * @param vaddr Virtual address to check
  *
  * @return bool true if address is within protected region, false otherwise
  */
@@ -89,7 +89,7 @@ bool esp_tee_flash_check_vaddr_in_tee_region(const size_t vaddr);
 /**
  * @brief Check if the given physical address falls within the TEE flash protected region
  *
- * @param addr Physical address to check
+ * @param paddr Physical address to check
  *
  * @return bool true if address is within protected region, false otherwise
  */
@@ -98,8 +98,38 @@ bool esp_tee_flash_check_paddr_in_tee_region(const size_t paddr);
 /**
  * @brief Check if the given physical address falls within the active TEE partition
  *
- * @param dest_addr Physical address to check
+ * @param paddr Physical address to check
  *
  * @return bool true if address is within active TEE partition, false otherwise
  */
 bool esp_tee_flash_check_paddr_in_active_tee_part(const size_t paddr);
+
+/**
+ * @brief Check if the given virtual address range overlaps with TEE flash protected regions
+ *
+ * @param vaddr Starting virtual address of the range to check
+ * @param len   Length of the address range in bytes
+ *
+ * @return bool true if any part of the range overlaps with protected regions, false otherwise
+ */
+bool esp_tee_flash_check_vrange_in_tee_region(const size_t vaddr, const size_t len);
+
+/**
+ * @brief Check if the given physical address range overlaps with TEE flash protected region
+ *
+ * @param paddr Starting physical address of the range to check
+ * @param len   Length of the address range in bytes
+ *
+ * @return bool true if any part of the range overlaps with TEE protected region, false otherwise
+ */
+bool esp_tee_flash_check_prange_in_tee_region(const size_t paddr, const size_t len);
+
+/**
+ * @brief Check if the given physical address range overlaps with active TEE partition
+ *
+ * @param paddr Starting physical address of the range to check
+ * @param len   Length of the address range in bytes
+ *
+ * @return bool true if any part of the range overlaps with active TEE partition, false otherwise
+ */
+bool esp_tee_flash_check_prange_in_active_tee_part(const size_t paddr, const size_t len);

components/esp_tee/subproject/components/tee_sec_storage/tee_sec_storage.c

@@ -644,7 +644,11 @@ esp_err_t esp_tee_sec_storage_ecdsa_sign_pbkdf2(const esp_tee_sec_storage_pbkdf2
     }
 
     hmac_key_id_t key_id = (hmac_key_id_t)(CONFIG_SECURE_TEE_PBKDF2_EFUSE_HMAC_KEY_ID);
-    esp_efuse_block_t blk = EFUSE_BLK_KEY0 + (esp_efuse_block_t)(key_id);
+    if (key_id < 0 || key_id >= HMAC_KEY_MAX) {
+        return ESP_ERR_INVALID_ARG;
+    }
+
+    esp_efuse_block_t blk = (esp_efuse_block_t)(EFUSE_BLK_KEY0 + key_id);
     if (esp_efuse_get_key_purpose(blk) != ESP_EFUSE_KEY_PURPOSE_HMAC_UP) {
         ESP_LOGE(TAG, "HMAC key is not burnt in the specified eFuse block ID");
         return ESP_ERR_NOT_FOUND;

components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors_plic.S

@@ -23,7 +23,9 @@
     .equ RTNVAL, 0xc0de
     .equ ECALL_U_MODE, 0x8
     .equ ECALL_M_MODE, 0xb
-    .equ TEE_INTR_DELEG_MASK, ~(1U << TEE_SECURE_INUM)
+    /* NOTE: INTWDT timeout and Cache error interrupts trigger the panic
+     * handler before reset, so they don’t need to be delegated. */
+    .equ TEE_INTR_DELEG_MASK, ~((1U << TEE_SECURE_INUM) | (1U << ETS_INT_WDT_INUM) | (1U << ETS_CACHEERR_INUM))
 
     .global esp_tee_global_interrupt_handler
     .global esp_tee_service_dispatcher

components/esp_tee/subproject/main/core/esp_secure_services_iram.c

@@ -219,8 +219,8 @@ esp_err_t _ss_esp_tee_sec_storage_ecdsa_sign_pbkdf2(const esp_tee_sec_storage_pb
 void _ss_mmu_hal_map_region(uint32_t mmu_id, mmu_target_t mem_type, uint32_t vaddr,
                             uint32_t paddr, uint32_t len, uint32_t *out_len)
 {
-    bool vaddr_chk = esp_tee_flash_check_vaddr_in_tee_region(vaddr);
-    bool paddr_chk = esp_tee_flash_check_paddr_in_tee_region(paddr);
+    bool vaddr_chk = esp_tee_flash_check_vrange_in_tee_region(vaddr, len);
+    bool paddr_chk = esp_tee_flash_check_prange_in_tee_region(paddr, len);
     if (vaddr_chk || paddr_chk) {
         ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x | 0x%08x", __func__, vaddr, paddr);
         return;
@@ -232,7 +232,7 @@ void _ss_mmu_hal_map_region(uint32_t mmu_id, mmu_target_t mem_type, uint32_t vad
 
 void _ss_mmu_hal_unmap_region(uint32_t mmu_id, uint32_t vaddr, uint32_t len)
 {
-    bool vaddr_chk = esp_tee_flash_check_vaddr_in_tee_region(vaddr);
+    bool vaddr_chk = esp_tee_flash_check_vrange_in_tee_region(vaddr, len);
     if (vaddr_chk) {
         ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, vaddr);
         return;
@@ -279,7 +279,8 @@ uint32_t _ss_spi_flash_hal_check_status(spi_flash_host_inst_t *host)
 
 esp_err_t _ss_spi_flash_hal_common_command(spi_flash_host_inst_t *host, spi_flash_trans_t *trans)
 {
-    bool paddr_chk = esp_tee_flash_check_paddr_in_tee_region(trans->address);
+    bool paddr_chk = esp_tee_flash_check_prange_in_tee_region(trans->address, trans->mosi_len);
+    paddr_chk |= esp_tee_flash_check_prange_in_tee_region(trans->address, trans->miso_len);
     if (paddr_chk) {
         ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, trans->address);
         return ESP_FAIL;
@@ -303,14 +304,9 @@ void _ss_spi_flash_hal_erase_block(spi_flash_host_inst_t *host, uint32_t start_a
     spi_flash_hal_erase_block(host, start_address);
 }
 
-void _ss_spi_flash_hal_erase_chip(spi_flash_host_inst_t *host)
-{
-    spi_flash_hal_erase_chip(host);
-}
-
 void _ss_spi_flash_hal_erase_sector(spi_flash_host_inst_t *host, uint32_t start_address)
 {
-    bool paddr_chk = esp_tee_flash_check_paddr_in_tee_region(start_address);
+    bool paddr_chk = esp_tee_flash_check_prange_in_tee_region(start_address, FLASH_SECTOR_SIZE);
     if (paddr_chk) {
         ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, start_address);
         return;
@@ -321,7 +317,7 @@ void _ss_spi_flash_hal_erase_sector(spi_flash_host_inst_t *host, uint32_t start_
 
 void _ss_spi_flash_hal_program_page(spi_flash_host_inst_t *host, const void *buffer, uint32_t address, uint32_t length)
 {
-    bool paddr_chk = esp_tee_flash_check_paddr_in_tee_region(address);
+    bool paddr_chk = esp_tee_flash_check_prange_in_tee_region(address, length);
     if (paddr_chk) {
         ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, address);
         return;
@@ -338,7 +334,7 @@ void _ss_spi_flash_hal_program_page(spi_flash_host_inst_t *host, const void *buf
 
 esp_err_t _ss_spi_flash_hal_read(spi_flash_host_inst_t *host, void *buffer, uint32_t address, uint32_t read_len)
 {
-    bool paddr_chk = esp_tee_flash_check_paddr_in_tee_region(address);
+    bool paddr_chk = esp_tee_flash_check_prange_in_tee_region(address, read_len);
     if (paddr_chk) {
         ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, address);
         return ESP_FAIL;
@@ -396,10 +392,11 @@ uint32_t _ss_bootloader_flash_execute_command_common(
     uint8_t mosi_len, uint32_t mosi_data,
     uint8_t miso_len)
 {
-    bool paddr_chk = esp_tee_flash_check_paddr_in_tee_region(address);
+    bool paddr_chk = esp_tee_flash_check_prange_in_tee_region(address, mosi_len);
+    paddr_chk |= esp_tee_flash_check_prange_in_tee_region(address, miso_len);
     if (paddr_chk) {
         ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, address);
-        return ESP_FAIL;
+        return 0;
     }
     ESP_FAULT_ASSERT(!paddr_chk);
     return bootloader_flash_execute_command_common(command, addr_len, address, dummy_len,
@@ -408,7 +405,7 @@ uint32_t _ss_bootloader_flash_execute_command_common(
 
 esp_err_t _ss_memspi_host_flush_cache(spi_flash_host_inst_t *host, uint32_t addr, uint32_t size)
 {
-    bool paddr_chk = esp_tee_flash_check_paddr_in_tee_region(addr);
+    bool paddr_chk = esp_tee_flash_check_prange_in_tee_region(addr, size);
     if (paddr_chk) {
         return ESP_FAIL;
     }

components/esp_tee/test_apps/tee_test_fw/main/test_esp_tee_flash_prot.c

@@ -250,6 +250,16 @@ TEST_CASE_MULTIPLE_STAGES("Test REE-TEE isolation: Flash - SPI0 (spi_flash_mmap)
                           test_initial_boot, test_spi_flash_mmap_api, test_spi_flash_mmap_api,
                           test_spi_flash_mmap_api);
 
+TEST_CASE("Test REE-TEE isolation: MMU-spillover", "[exception]")
+{
+    const void *ptr;
+    spi_flash_mmap_handle_t handle;
+    const size_t len = 0x100000;  // 1MB
+    TEST_ESP_OK(spi_flash_mmap(0x00, len, SPI_FLASH_MMAP_DATA, &ptr, &handle));
+    ESP_LOG_BUFFER_HEXDUMP(TAG, ptr, 32, ESP_LOG_INFO);
+    TEST_FAIL_MESSAGE("Exception should have been generated!");
+}
+
 /* ---------------------------------------------- API family 3: esp_flash ------------------------------------------------- */
 
 #if CONFIG_SECURE_TEE_EXT_FLASH_MEMPROT_SPI1

components/esp_tee/test_apps/tee_test_fw/pytest_esp_tee_ut.py

@@ -51,6 +51,7 @@ REE_ISOLATION_TEST_EXC_RSN: dict[str, str] = {
     ('IROM-W1'): 'Store access fault',
     ('DROM-R1'): 'Load access fault',
     ('DROM-W1'): 'Store access fault',
+    ('MMU-spillover'): 'Cache error',
 }
 
 TEE_APM_VIOLATION_EXC_CHK = ['eFuse', 'MMU', 'AES', 'HMAC', 'DS', 'SHA PCR', 'ECC PCR', 'SWDT/BOD']

components/mbedtls/test_apps/main/test_sha.c

@@ -117,6 +117,12 @@ TEST_CASE("Test esp_sha()", "[hw_crypto]")
 #endif
 }
 
+/* NOTE: This test attempts to mmap 1MB of flash starting from address 0x00, which overlaps
+ * the entire TEE protected region, causing the mmap operation to fail and triggering an
+ * exception in the subsequent steps.
+ */
+#if !CONFIG_SECURE_ENABLE_TEE
+
 TEST_CASE("Test esp_sha() function with long input", "[hw_crypto]")
 {
     int r = -1;
@@ -173,6 +179,7 @@ TEST_CASE("Test esp_sha() function with long input", "[hw_crypto]")
 #endif
 }
 
+#endif
 
 #if CONFIG_MBEDTLS_HARDWARE_SHA