From 304adb120b0ba9382cb7e7b12db1e2e3a49cf17a Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Mon, 8 Dec 2025 16:30:17 +0530 Subject: [PATCH] refactor(esp_tee): Adopt `Picolibc` as the default LibC for ESP-TEE build - Also fixed an issue where NewLib ROM APIs, when called from TEE, were using the syscall table located in the REE SRAM. This could be abused as an attack vector to invoke illegal functions from the TEE. To prevent this, the syscall table is now switched to the TEE-specific copy during every M-U mode transition. --- .../esp_libc/src/picolibc/picolibc_init.c | 47 ----------- components/esp_tee/Kconfig.projbuild | 14 ++++ components/esp_tee/project_include.cmake | 35 ++++++++- .../main/arch/riscv/esp_tee_vectors_clic.S | 10 +++ .../main/arch/riscv/esp_tee_vectors_plic.S | 10 +++ .../subproject/main/common/multi_heap.c | 4 +- .../subproject/main/common/syscall_stubs.c | 77 +++++++++++++++++++ .../subproject/main/ld/esp32c5/esp_tee.ld.in | 22 ++++++ .../subproject/main/ld/esp32c6/esp_tee.ld.in | 22 ++++++ .../subproject/main/ld/esp32c61/esp_tee.ld.in | 22 ++++++ .../subproject/main/ld/esp32h2/esp_tee.ld.in | 22 ++++++ .../tee_test_fw/sdkconfig.ci.tee_ota | 4 +- 12 files changed, 235 insertions(+), 54 deletions(-) diff --git a/components/esp_libc/src/picolibc/picolibc_init.c b/components/esp_libc/src/picolibc/picolibc_init.c index 2d7f6c6855e..0756627ef42 100644 --- a/components/esp_libc/src/picolibc/picolibc_init.c +++ b/components/esp_libc/src/picolibc/picolibc_init.c @@ -37,55 +37,8 @@ int __retarget_lock_try_acquire(struct __lock * p); int __retarget_lock_try_acquire_recursive(struct __lock *p); #endif -#if CONFIG_SECURE_ENABLE_TEE -struct _reent_stub { - int _errno; - __FILE *_stdin, *_stdout, *_stderr; - int _inc; - char *_emergency; - int _reserved_0; - int _reserved_1; - struct __locale_t *_locale; - void *_mp; - void (*__cleanup)(struct _reent *); - int _gamma_signgam; - int _cvtlen; - char *_cvtbuf; - struct _rand48 *_r48; -#if 0 /* unlikely used fields in ROM implementation */ - struct __tm *_localtime_buf; - char *_asctime_buf; - void (** _sig_func)(int); - struct _atexit *_reserved_6; - struct _atexit _reserved_7; - struct _glue _reserved_8; - __FILE *__sf; - struct _misc_reent *_misc; - char *_signal_buf; -#endif -}; - -void *__getreent_rom_stub(void) -{ - static struct _reent_stub reent_stub; - return &reent_stub; -} -#endif // SECURE_ENABLE_TEE - static struct syscall_stub_table s_stub_table = { -#if CONFIG_SECURE_ENABLE_TEE - /* - * ESP-TEE uses snprintf() from ROM, which requires at least a fake __getreent stub. - * - * NOTE: If floating-point variables are intended to be used, - * the following fields must be specified in the syscall_stub_table: - * ._printf_float = - * ._scanf_float = - */ - .__getreent = (void *)__getreent_rom_stub, -#else .__getreent = (void *)abort, -#endif ._malloc_r = (void *)abort, ._free_r = (void *)abort, ._realloc_r = (void *)abort, diff --git a/components/esp_tee/Kconfig.projbuild b/components/esp_tee/Kconfig.projbuild index 14b7518734a..6cb5be8eb8e 100644 --- a/components/esp_tee/Kconfig.projbuild +++ b/components/esp_tee/Kconfig.projbuild @@ -158,6 +158,20 @@ menu "ESP-TEE (Trusted Execution Environment)" Note: All accesses to the TEE partitions over SPI0 (i.e. the MMU) are blocked unconditionally. + choice SECURE_TEE_LIBC + prompt "LibC to build the TEE application with" + depends on SECURE_ENABLE_TEE + default SECURE_TEE_LIBC_NEWLIB if IDF_TOOLCHAIN_CLANG + default SECURE_TEE_LIBC_PICOLIBC + + config SECURE_TEE_LIBC_PICOLIBC + bool "Picolibc" + depends on !IDF_TOOLCHAIN_CLANG + config SECURE_TEE_LIBC_NEWLIB + bool "NewLib" + + endchoice + config SECURE_TEE_DEBUG_MODE bool "Enable Debug Mode" default y diff --git a/components/esp_tee/project_include.cmake b/components/esp_tee/project_include.cmake index cb6fd8aa301..3358a14e56c 100644 --- a/components/esp_tee/project_include.cmake +++ b/components/esp_tee/project_include.cmake @@ -32,10 +32,41 @@ set(tee_binary_files "${TEE_BUILD_DIR}/esp_tee.map" ) -# Use only Newlib libc to reduce binary size, as some Newlib functions are already available in ROM +# Override LibC for ESP-TEE if needed set(esp_tee_sdkconfig "${CMAKE_CURRENT_BINARY_DIR}/sdkconfig.esp_tee") configure_file("${sdkconfig}" "${esp_tee_sdkconfig}" COPYONLY) -file(APPEND "${esp_tee_sdkconfig}" "\nCONFIG_LIBC_NEWLIB=y\n") + +file(READ "${esp_tee_sdkconfig}" content) + +unset(REE_LIBC) +unset(TEE_LIBC) + +foreach(libc NEWLIB PICOLIBC) + if(content MATCHES "CONFIG_LIBC_${libc}=y") + set(REE_LIBC ${libc}) + endif() + + if(content MATCHES "CONFIG_SECURE_TEE_LIBC_${libc}=y") + set(TEE_LIBC ${libc}) + endif() +endforeach() + +if(REE_LIBC AND TEE_LIBC AND NOT REE_LIBC STREQUAL TEE_LIBC) + string(REGEX REPLACE + "CONFIG_LIBC_(NEWLIB|PICOLIBC)=y" + "# CONFIG_LIBC_\\1 is not set" + content + "${content}" + ) + # Enable libc selected by TEE + string(REGEX REPLACE + "# CONFIG_LIBC_${TEE_LIBC} is not set" + "CONFIG_LIBC_${TEE_LIBC}=y" + content + "${content}" + ) + file(WRITE "${esp_tee_sdkconfig}" "${content}") +endif() set(secure_service_headers_dir "${CMAKE_CURRENT_BINARY_DIR}/secure_service_headers") make_directory(${secure_service_headers_dir}) diff --git a/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors_clic.S b/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors_clic.S index 8dda887c97d..5a5e266e189 100644 --- a/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors_clic.S +++ b/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors_clic.S @@ -34,6 +34,8 @@ .global esp_tee_global_interrupt_handler .global esp_tee_service_dispatcher .global _tee_s_entry + .global syscall_enter_tee + .global syscall_exit_tee .section .data .balign 4 @@ -392,6 +394,8 @@ _skip_thresh_restore: STACK_GUARD_POST_SWITCH t3 1 _ns_sp_min _ns_sp_max fence + call syscall_exit_tee + /* Backup the A0 register * This point is reached after an ecall is triggered after executing the secure service. * The A0 register contains the return value of the corresponding service. @@ -464,6 +468,8 @@ _2: li t0, MSTATUS_MPP csrs mstatus, t0 + call syscall_enter_tee + mret /* This point is reached after servicing a U-mode interrupt occurred @@ -491,6 +497,8 @@ _3: li t0, MSTATUS_MPP csrs mstatus, t0 + call syscall_enter_tee + /* Restore register context and resume the secure service */ restore_mepc restore_general_regs @@ -615,6 +623,8 @@ _4: STACK_GUARD_POST_SWITCH t3 1 _ns_sp_min _ns_sp_max + call syscall_exit_tee + /* Place magic bytes in all the general registers */ store_magic_general_regs diff --git a/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors_plic.S b/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors_plic.S index fcaf3a76ff8..52d4728dfaa 100644 --- a/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors_plic.S +++ b/components/esp_tee/subproject/main/arch/riscv/esp_tee_vectors_plic.S @@ -39,6 +39,8 @@ .global esp_tee_global_interrupt_handler .global esp_tee_service_dispatcher .global _tee_s_entry + .global syscall_enter_tee + .global syscall_exit_tee .section .data .balign 4 @@ -384,6 +386,8 @@ _skip_thresh_restore: STACK_GUARD_POST_SWITCH t3 1 _ns_sp_min _ns_sp_max fence + call syscall_exit_tee + /* Backup the A0 register * This point is reached after an ecall is triggered after executing the secure service. * The A0 register contains the return value of the corresponding service. @@ -452,6 +456,8 @@ _process_ecall: li t0, MSTATUS_MPP csrs mstatus, t0 + call syscall_enter_tee + mret /* This point is reached after servicing a U-mode interrupt occurred @@ -472,6 +478,8 @@ _rtn_from_ns_int: li t0, MSTATUS_MPP csrs mstatus, t0 + call syscall_enter_tee + /* Restore register context and resume the secure service */ restore_mepc restore_general_regs @@ -545,6 +553,8 @@ _tee_ns_intr_handler: STACK_GUARD_POST_SWITCH t3 1 _ns_sp_min _ns_sp_max + call syscall_exit_tee + /* Place magic bytes in all the general registers */ store_magic_general_regs diff --git a/components/esp_tee/subproject/main/common/multi_heap.c b/components/esp_tee/subproject/main/common/multi_heap.c index 49d963a8e65..5a654dd7a6c 100644 --- a/components/esp_tee/subproject/main/common/multi_heap.c +++ b/components/esp_tee/subproject/main/common/multi_heap.c @@ -152,7 +152,6 @@ void *calloc(size_t n, size_t size) return esp_tee_heap_calloc(n, size); } -#if CONFIG_LIBC_PICOLIBC void *realloc(void* ptr, size_t size) { if (tee_heap == NULL) { @@ -160,7 +159,7 @@ void *realloc(void* ptr, size_t size) } if (ptr == NULL) { - return esp_tee_heap_malloc(heap, size); + return esp_tee_heap_malloc(size); } size_t previous_block_size = tlsf_block_size(ptr); @@ -177,7 +176,6 @@ void *realloc(void* ptr, size_t size) return result; } -#endif void free(void *ptr) { diff --git a/components/esp_tee/subproject/main/common/syscall_stubs.c b/components/esp_tee/subproject/main/common/syscall_stubs.c index 9ed7e86cf5b..5e667f2ebee 100644 --- a/components/esp_tee/subproject/main/common/syscall_stubs.c +++ b/components/esp_tee/subproject/main/common/syscall_stubs.c @@ -14,7 +14,13 @@ #include #include +#include "soc/soc_caps.h" +#include "rom/libc_stubs.h" + +#include "esp_attr.h" +#include "esp_cpu.h" #include "esp_random.h" + #include "sdkconfig.h" #if CONFIG_LIBC_NEWLIB @@ -76,7 +82,44 @@ int _getentropy_r(struct _reent *r, void *buffer, size_t length) esp_fill_random(buffer, length); return 0; } + +struct syscall_stub_table *syscall_table_ptr; +static struct syscall_stub_table *syscall_table_ptr_ree; + +static struct syscall_stub_table s_stub_table_tee = { + .__getreent = &__getreent, +}; + +void IRAM_ATTR syscall_enter_tee(void) +{ + if (syscall_table_ptr) { + syscall_table_ptr_ree = syscall_table_ptr; + syscall_table_ptr = &s_stub_table_tee; + } +} + +void IRAM_ATTR syscall_exit_tee(void) +{ + if (syscall_table_ptr_ree) { + syscall_table_ptr = syscall_table_ptr_ree; + syscall_table_ptr_ree = NULL; + } +} #else +/* + * Picolibc does not initialize 'errno' and places it in the TBSS section. + * + * To allow convenient initialization and support interoperability with Newlib, + * 'errno' is defined in the TDATA section. The linker script ensures that + * it is positioned at the beginning of the TDATA segment. + */ +__thread int errno __attribute__((section(".tdata.errno"))) = 0; + +int *__errno(void) +{ + return &errno; +} + int fstat(int fd, struct stat *st) { errno = ENOSYS; @@ -123,6 +166,40 @@ int getentropy(void *buffer, size_t length) esp_fill_random(buffer, length); return 0; } + +int open(const char *path, int flags, ...) +{ + errno = ENOSYS; + return -1; +} + +int rename(const char *src, const char *dst) +{ + errno = ENOSYS; + return -1; +} + +int unlink(const char *path) +{ + errno = ENOSYS; + return -1; +} + +extern uint32_t _tee_tdata_start; +static void *s_tp_ree[SOC_CPU_CORES_NUM]; + +void IRAM_ATTR syscall_enter_tee(void) +{ + int core_id = esp_cpu_get_core_id(); + s_tp_ree[core_id] = esp_cpu_get_threadptr(); + esp_cpu_set_threadptr(&_tee_tdata_start); +} + +void IRAM_ATTR syscall_exit_tee(void) +{ + int core_id = esp_cpu_get_core_id(); + esp_cpu_set_threadptr(s_tp_ree[core_id]); +} #endif // CONFIG_LIBC_NEWLIB void *pthread_getspecific(pthread_key_t key) diff --git a/components/esp_tee/subproject/main/ld/esp32c5/esp_tee.ld.in b/components/esp_tee/subproject/main/ld/esp32c5/esp_tee.ld.in index d0742e27a19..17be36c89e2 100644 --- a/components/esp_tee/subproject/main/ld/esp32c5/esp_tee.ld.in +++ b/components/esp_tee/subproject/main/ld/esp32c5/esp_tee.ld.in @@ -132,6 +132,28 @@ SECTIONS ALIGNED_SYMBOL(8, _tee_bss_end) } > sram_tee_seg + .dram.tee.tdata : + { + /* tdata sections */ + _tee_tdata_start = ABSOLUTE(.); +#if CONFIG_LIBC_PICOLIBC + KEEP(*(.tdata.errno)) +#endif // CONFIG_LIBC_PICOLIBC + *(.tdata .tdata.* .gnu.linkonce.td.*) + _tee_tdata_end = ABSOLUTE(.); + + /* tbss sections */ + _tee_tbss_start = ABSOLUTE(.); + *(.tbss .tbss.* .gnu.linkonce.tb.*) + *(.tcommon .tcommon.*) + _tee_tbss_end = ABSOLUTE(.); + + . = ALIGN(ALIGNOF(.dram.tee.rodata)); + } > sram_tee_seg + ASSERT_SECTIONS_GAP(.dram.tee.tdata, .dram.tee.rodata) + ASSERT(_tee_tdata_end == _tee_tbss_start, + "tdata and tbss must be contiguous.") + .dram.tee.rodata : { _tee_rodata_start = ABSOLUTE(.); diff --git a/components/esp_tee/subproject/main/ld/esp32c6/esp_tee.ld.in b/components/esp_tee/subproject/main/ld/esp32c6/esp_tee.ld.in index 2c9ed0d9917..999e5faa7d2 100644 --- a/components/esp_tee/subproject/main/ld/esp32c6/esp_tee.ld.in +++ b/components/esp_tee/subproject/main/ld/esp32c6/esp_tee.ld.in @@ -137,6 +137,28 @@ SECTIONS ALIGNED_SYMBOL(8, _tee_bss_end) } > sram_tee_seg + .dram.tee.tdata : + { + /* tdata sections */ + _tee_tdata_start = ABSOLUTE(.); +#if CONFIG_LIBC_PICOLIBC + KEEP(*(.tdata.errno)) +#endif // CONFIG_LIBC_PICOLIBC + *(.tdata .tdata.* .gnu.linkonce.td.*) + _tee_tdata_end = ABSOLUTE(.); + + /* tbss sections */ + _tee_tbss_start = ABSOLUTE(.); + *(.tbss .tbss.* .gnu.linkonce.tb.*) + *(.tcommon .tcommon.*) + _tee_tbss_end = ABSOLUTE(.); + + . = ALIGN(ALIGNOF(.dram.tee.rodata)); + } > sram_tee_seg + ASSERT_SECTIONS_GAP(.dram.tee.tdata, .dram.tee.rodata) + ASSERT(_tee_tdata_end == _tee_tbss_start, + "tdata and tbss must be contiguous.") + .dram.tee.rodata : { _tee_rodata_start = ABSOLUTE(.); diff --git a/components/esp_tee/subproject/main/ld/esp32c61/esp_tee.ld.in b/components/esp_tee/subproject/main/ld/esp32c61/esp_tee.ld.in index ac4feedefd8..9627ff26939 100644 --- a/components/esp_tee/subproject/main/ld/esp32c61/esp_tee.ld.in +++ b/components/esp_tee/subproject/main/ld/esp32c61/esp_tee.ld.in @@ -132,6 +132,28 @@ SECTIONS ALIGNED_SYMBOL(8, _tee_bss_end) } > sram_tee_seg + .dram.tee.tdata : + { + /* tdata sections */ + _tee_tdata_start = ABSOLUTE(.); +#if CONFIG_LIBC_PICOLIBC + KEEP(*(.tdata.errno)) +#endif // CONFIG_LIBC_PICOLIBC + *(.tdata .tdata.* .gnu.linkonce.td.*) + _tee_tdata_end = ABSOLUTE(.); + + /* tbss sections */ + _tee_tbss_start = ABSOLUTE(.); + *(.tbss .tbss.* .gnu.linkonce.tb.*) + *(.tcommon .tcommon.*) + _tee_tbss_end = ABSOLUTE(.); + + . = ALIGN(ALIGNOF(.dram.tee.rodata)); + } > sram_tee_seg + ASSERT_SECTIONS_GAP(.dram.tee.tdata, .dram.tee.rodata) + ASSERT(_tee_tdata_end == _tee_tbss_start, + "tdata and tbss must be contiguous.") + .dram.tee.rodata : { _tee_rodata_start = ABSOLUTE(.); diff --git a/components/esp_tee/subproject/main/ld/esp32h2/esp_tee.ld.in b/components/esp_tee/subproject/main/ld/esp32h2/esp_tee.ld.in index 9c42a51038d..8bfecfc59f3 100644 --- a/components/esp_tee/subproject/main/ld/esp32h2/esp_tee.ld.in +++ b/components/esp_tee/subproject/main/ld/esp32h2/esp_tee.ld.in @@ -137,6 +137,28 @@ SECTIONS ALIGNED_SYMBOL(8, _tee_bss_end) } > sram_tee_seg + .dram.tee.tdata : + { + /* tdata sections */ + _tee_tdata_start = ABSOLUTE(.); +#if CONFIG_LIBC_PICOLIBC + KEEP(*(.tdata.errno)) +#endif // CONFIG_LIBC_PICOLIBC + *(.tdata .tdata.* .gnu.linkonce.td.*) + _tee_tdata_end = ABSOLUTE(.); + + /* tbss sections */ + _tee_tbss_start = ABSOLUTE(.); + *(.tbss .tbss.* .gnu.linkonce.tb.*) + *(.tcommon .tcommon.*) + _tee_tbss_end = ABSOLUTE(.); + + . = ALIGN(ALIGNOF(.dram.tee.rodata)); + } > sram_tee_seg + ASSERT_SECTIONS_GAP(.dram.tee.tdata, .dram.tee.rodata) + ASSERT(_tee_tdata_end == _tee_tbss_start, + "tdata and tbss must be contiguous.") + .dram.tee.rodata : { _tee_rodata_start = ABSOLUTE(.); diff --git a/components/esp_tee/test_apps/tee_test_fw/sdkconfig.ci.tee_ota b/components/esp_tee/test_apps/tee_test_fw/sdkconfig.ci.tee_ota index 13a1bc558d4..04e7eb42530 100644 --- a/components/esp_tee/test_apps/tee_test_fw/sdkconfig.ci.tee_ota +++ b/components/esp_tee/test_apps/tee_test_fw/sdkconfig.ci.tee_ota @@ -19,5 +19,5 @@ CONFIG_SECURE_TEE_ATT_KEY_STR_ID="tee_att_keyN" CONFIG_SECURE_TEE_EXT_FLASH_MEMPROT_SPI1=y # Increasing TEE DRAM size -# 19KB -CONFIG_SECURE_TEE_DRAM_SIZE=0x4c00 +# 20KB +CONFIG_SECURE_TEE_DRAM_SIZE=0x5000