From 0964024484a0798258c081609e8bda36feac8bca 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 2d7f6c6855..0756627ef4 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 14b7518734..6cb5be8eb8 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 cb6fd8aa30..3358a14e56 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 789188e2cb..79d1135cb1 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 9fd6f5ec98..2684e6afc4 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 49d963a8e6..5a654dd7a6 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 9ed7e86cf5..5e667f2ebe 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 d0742e27a1..17be36c89e 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 2c9ed0d991..999e5faa7d 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 ac4feedefd..9627ff2693 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 9c42a51038..8bfecfc59f 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 13a1bc558d..04e7eb4253 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