From b6fe9c1e490a7d13110f34c21e163bab70e6b068 Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Thu, 2 Jul 2026 22:32:10 +0000 Subject: [PATCH 1/5] satisfy the OP-TEE TA stack-guard read via the stack canary --- litebox_shim_optee/src/lib.rs | 84 +++++++++-------------- litebox_shim_optee/src/loader/ta_stack.rs | 39 ++++++++++- 2 files changed, 69 insertions(+), 54 deletions(-) diff --git a/litebox_shim_optee/src/lib.rs b/litebox_shim_optee/src/lib.rs index c956074e6..6f5ee7272 100644 --- a/litebox_shim_optee/src/lib.rs +++ b/litebox_shim_optee/src/lib.rs @@ -269,8 +269,6 @@ impl OpteeShim { ta_entry_point: Cell::new(0), ta_stack_base_addr: Cell::new(0), ta_prepared: Cell::new(false), - #[cfg(target_arch = "x86_64")] - tls_base_addr: Cell::new(0), }, }; if let Some(ta_bin) = ta_bin @@ -783,15 +781,9 @@ impl Task { let ta_entry_point = self.get_ta_entry_point(); let mut elf_loader = loader::elf::ElfLoader::new(self, &ta_bin, false)?; elf_loader.load_ta_trampoline(ta_entry_point)?; - self.allocate_guest_tls(None).map_err(|_| { - ElfLoaderError::MappingError(litebox::mm::linux::MappingError::OutOfMemory) - })?; self.ta_prepared.set(true); } - #[cfg(target_arch = "x86_64")] - self.restore_guest_tls(); - let mut ta_stack = crate::loader::ta_stack::allocate_stack(self, self.get_ta_stack_base_addr()).ok_or( ElfLoaderError::MappingError(litebox::mm::linux::MappingError::OutOfMemory), @@ -800,6 +792,18 @@ impl Task { .init(self.global.platform, params) .ok_or(ElfLoaderError::InvalidStackAddr)?; + // Point the FS base at the TA stack canary so the compiler's + // stack-guard reads resolve to it. Must run after + // `ta_stack.init` (which pushes the canary) and on every entry, + // because the guest FS base does not persist across shim<->guest + // transitions (see `set_guest_stack_guard_fs_base`). + #[cfg(target_arch = "x86_64")] + Self::set_guest_stack_guard_fs_base( + ta_stack + .canary_addr() + .ok_or(ElfLoaderError::InvalidStackAddr)?, + )?; + Ok(ThreadInitState::Ta { cmd_id: cmd_id.unwrap_or(0) as usize, params_address: ta_stack.get_params_address(), @@ -839,52 +843,31 @@ impl Task { } } - /// Allocate the guest TLS for an OP-TEE TA. + /// Point the guest FS base at the TA stack canary so the compiler's stack + /// guard read resolves onto it. /// - /// This function is required to overcome the compatibility issue coming from - /// system and build toolchain differences. OP-TEE OS only supports a single thread and - /// thus does not explicitly set up the TLS area. In contrast, we do use an x86 toolchain to - /// compile OP-TEE TAs and this toolchain assumes there is a valid TLS areas for various purposes - /// including stack protection. To this end, the toolchain generates binaries using - /// the `FS` register for TLS access. - /// This function allocates a TLS area on behalf of the TA to satisfy the toolchain's assumption. - /// Instead of using this function, we could change the flags of the toolchain to not use TLS - /// (e.g., `-fno-stack-protector`), but this might be insecure. Also, the toolchain might have - /// other features relying on TLS. - #[cfg(target_arch = "x86_64")] - fn allocate_guest_tls( - &self, - tls_size: Option, - ) -> Result<(), litebox_common_linux::errno::Errno> { - let tls_size = tls_size.unwrap_or(PAGE_SIZE).next_multiple_of(PAGE_SIZE); - let addr = self.sys_mmap( - 0, - tls_size, - ProtFlags::PROT_READ | ProtFlags::PROT_WRITE, - MapFlags::MAP_PRIVATE | MapFlags::MAP_ANONYMOUS, - -1, - 0, - )?; - // Store TLS address for later restoration - self.tls_base_addr.set(addr.as_usize()); - self.restore_guest_tls(); - Ok(()) - } - - /// Restore the guest TLS (FS base) before entering the TA. + /// OP-TEE TAs are single-threaded and have no TLS block, but the x86-64 + /// toolchain still emits stack-protector reads of `%fs:0x28`. To satisfy + /// those reads without allocating a TLS area, we set the FS base to + /// `canary_addr - ABI_STACK_GUARD_FS_OFFSET` (an ABI-fixed offset, not a + /// tunable), so the compiler's guard read lands on the TA stack canary. /// - /// FS base is cleared across VTL switches, so we must restore it before - /// every TA entry. + /// This must be called on every TA entry: the guest FS base is not + /// guaranteed to persist across a shim<->guest transition. Re-establishing + /// it here keeps the shim platform-agnostic. + /// + /// Returns [`ElfLoaderError::InvalidStackAddr`] if `canary_addr` is below + /// `ABI_STACK_GUARD_FS_OFFSET`. #[cfg(target_arch = "x86_64")] - fn restore_guest_tls(&self) { + fn set_guest_stack_guard_fs_base(canary_addr: usize) -> Result<(), ElfLoaderError> { use litebox::platform::ArchSpecificProvider as _; - let addr = self.tls_base_addr.get(); - if addr == 0 { - return; // TLS not allocated yet - } + let fs_base = canary_addr + .checked_sub(crate::loader::ta_stack::ABI_STACK_GUARD_FS_OFFSET) + .ok_or(ElfLoaderError::InvalidStackAddr)?; litebox_platform_multiplex::platform() - .set_arch_specific_register(&litebox::platform::ArchSpecificRegister::FsBase, addr) + .set_arch_specific_register(&litebox::platform::ArchSpecificRegister::FsBase, fs_base) .expect("requires guaranteed platform support for FsBase"); + Ok(()) } /// Retrieve the result of the `ldelf` execution. @@ -1363,9 +1346,6 @@ struct Task { ta_stack_base_addr: Cell, /// Whether the TA has been prepared ta_prepared: Cell, - /// TLS base address for x86_64 (stored to restore FS before each TA entry) - #[cfg(target_arch = "x86_64")] - tls_base_addr: Cell, // TODO: OP-TEE supports global, persistent objects across sessions. Add these maps if needed. } @@ -1530,8 +1510,6 @@ mod test_utils { ta_entry_point: Cell::new(0), ta_stack_base_addr: Cell::new(0), ta_prepared: Cell::new(false), - #[cfg(target_arch = "x86_64")] - tls_base_addr: Cell::new(0), } } } diff --git a/litebox_shim_optee/src/loader/ta_stack.rs b/litebox_shim_optee/src/loader/ta_stack.rs index a0057a78b..d36477628 100644 --- a/litebox_shim_optee/src/loader/ta_stack.rs +++ b/litebox_shim_optee/src/loader/ta_stack.rs @@ -12,6 +12,22 @@ use zerocopy::IntoBytes; use crate::{Platform, UserMutPtr}; +/// ABI-mandated offset of the stack-protector guard from the x86-64 thread +/// pointer (`%fs`). This is a fixed ABI constant, **not** a tunable. +/// +/// On x86-64, GCC and Clang hardcode the stack-guard access as `%fs:0x28` when +/// using the default TLS-based stack protector. This offset is a fixed part of +/// that codegen ABI, independent of the C library. +/// +/// The guard itself is a single pointer-sized word, i.e., **8 bytes** on x86-64. +/// The read therefore spans `[%fs:0x28 .. %fs:0x30)`. +/// +/// OP-TEE TAs have no TLS block, so the shim sets the guest FS base to +/// `canary_addr - ABI_STACK_GUARD_FS_OFFSET`, making that read resolve onto the +/// CRNG canary pushed by [`TaStack::init`]. +#[cfg(target_arch = "x86_64")] +pub(crate) const ABI_STACK_GUARD_FS_OFFSET: usize = 0x28; + #[inline] fn align_down(addr: usize, align: usize) -> usize { debug_assert!(align.is_power_of_two()); @@ -62,6 +78,9 @@ pub struct TaStack { num_params: usize, /// Position where LdelfArg was pushed (if any) ldelf_arg_pos: Option, + #[cfg(target_arch = "x86_64")] + /// Position where the TA stack canary was pushed (if any). + canary_pos: Option, } impl TaStack { @@ -84,6 +103,7 @@ impl TaStack { params: UteeParams::new(), num_params: 0, ldelf_arg_pos: None, + canary_pos: None, }) } @@ -101,6 +121,13 @@ impl TaStack { self.stack_top.as_usize() + self.len - core::mem::size_of::() } + /// Get the address of the TA stack canary pushed by [`Self::init`]. + /// Returns `None` if no canary has been pushed yet. + #[cfg(target_arch = "x86_64")] + pub(crate) fn canary_addr(&self) -> Option { + self.canary_pos.map(|pos| self.stack_top.as_usize() + pos) + } + /// Get the address of `LdelfArg` on the stack. /// /// Returns the actual address where `LdelfArg` was pushed via `init_with_ldelf_arg`. @@ -124,6 +151,16 @@ impl TaStack { Some(()) } + /// Push the TA stack canary and record its address. + fn push_canary(&mut self, canary: &[u8; 16]) -> Option<()> { + self.push_bytes(canary)?; + #[cfg(target_arch = "x86_64")] + { + self.canary_pos = Some(self.pos); + } + Some(()) + } + /// Zero the unused stack region before a new session writes its parameters, /// to avoid leaking leftover data from a prior session whose stack region was recycled. /// The trailing `UteeParams` slot is left untouched here because @@ -259,7 +296,7 @@ impl TaStack { // Random 16-byte stack canary let mut canary = [0u8; 16]; ::fill_bytes_crng(platform, &mut canary); - self.push_bytes(&canary)?; + self.push_canary(&canary)?; // `reenter_thread` *jumps* into the TA entry point (which is a function) rather than // calls it. Adjust the stack pointer to ensure post-call stack alignment. From 7e7c9dff04e509c3fe58b0b33fd828db56da3033 Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Mon, 6 Jul 2026 21:56:10 +0000 Subject: [PATCH 2/5] fix wording --- litebox_shim_optee/src/lib.rs | 4 ++-- litebox_shim_optee/src/loader/ta_stack.rs | 12 +++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/litebox_shim_optee/src/lib.rs b/litebox_shim_optee/src/lib.rs index 6f5ee7272..995a65264 100644 --- a/litebox_shim_optee/src/lib.rs +++ b/litebox_shim_optee/src/lib.rs @@ -849,8 +849,8 @@ impl Task { /// OP-TEE TAs are single-threaded and have no TLS block, but the x86-64 /// toolchain still emits stack-protector reads of `%fs:0x28`. To satisfy /// those reads without allocating a TLS area, we set the FS base to - /// `canary_addr - ABI_STACK_GUARD_FS_OFFSET` (an ABI-fixed offset, not a - /// tunable), so the compiler's guard read lands on the TA stack canary. + /// `canary_addr - ABI_STACK_GUARD_FS_OFFSET` (shared by GCC, Clang, glibc, + /// and musl), so the compiler's guard read lands on the TA stack canary. /// /// This must be called on every TA entry: the guest FS base is not /// guaranteed to persist across a shim<->guest transition. Re-establishing diff --git a/litebox_shim_optee/src/loader/ta_stack.rs b/litebox_shim_optee/src/loader/ta_stack.rs index d36477628..287178240 100644 --- a/litebox_shim_optee/src/loader/ta_stack.rs +++ b/litebox_shim_optee/src/loader/ta_stack.rs @@ -12,12 +12,14 @@ use zerocopy::IntoBytes; use crate::{Platform, UserMutPtr}; -/// ABI-mandated offset of the stack-protector guard from the x86-64 thread -/// pointer (`%fs`). This is a fixed ABI constant, **not** a tunable. +/// Offset of the stack-protector guard from the x86-64 thread pointer (`%fs`). /// -/// On x86-64, GCC and Clang hardcode the stack-guard access as `%fs:0x28` when -/// using the default TLS-based stack protector. This offset is a fixed part of -/// that codegen ABI, independent of the C library. +/// This is a compiler + C-library convention: when using the default TLS-based +/// stack protector, both GCC and Clang emit the stack-guard access as +/// `%fs:0x28`, matching the `stack_guard` slot in the glibc/musl `tcbhead_t`. +/// The offset is nominally tunable via `-mstack-protector-guard-offset`, but +/// `0x28` is the fixed default across GCC, Clang, glibc, and musl on x86-64, +/// so we treat it as a constant here. /// /// The guard itself is a single pointer-sized word, i.e., **8 bytes** on x86-64. /// The read therefore spans `[%fs:0x28 .. %fs:0x30)`. From e84696280809939c0aee8365b95a54d5ee221d01 Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Sat, 11 Jul 2026 03:23:38 +0000 Subject: [PATCH 3/5] rebase --- litebox_shim_optee/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/litebox_shim_optee/src/lib.rs b/litebox_shim_optee/src/lib.rs index 995a65264..337cd754f 100644 --- a/litebox_shim_optee/src/lib.rs +++ b/litebox_shim_optee/src/lib.rs @@ -22,7 +22,7 @@ use litebox::{ shim::ContinueOperation, utils::TruncateExt, }; -use litebox_common_linux::{MapFlags, ProtFlags, errno::Errno, vmap::GlobalVmapManager}; +use litebox_common_linux::{errno::Errno, vmap::GlobalVmapManager}; use litebox_common_optee::{ LdelfArg, LdelfSyscallRequest, SyscallRequest, TaFlags, TeeAlgorithm, TeeAlgorithmClass, TeeAttributeType, TeeCrypStateHandle, TeeHandleFlag, TeeIdentity, TeeLogin, TeeObjHandle, From bd67673bd345a2bcf050f61cfc6c74089ef77bbb Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Wed, 15 Jul 2026 19:57:47 +0000 Subject: [PATCH 4/5] fix: isolate OP-TEE stack guard --- litebox_shim_optee/src/lib.rs | 72 ++++++++++++----------- litebox_shim_optee/src/loader/ta_stack.rs | 6 +- litebox_shim_optee/src/syscalls/tests.rs | 18 ++++++ 3 files changed, 59 insertions(+), 37 deletions(-) diff --git a/litebox_shim_optee/src/lib.rs b/litebox_shim_optee/src/lib.rs index 337cd754f..be55df954 100644 --- a/litebox_shim_optee/src/lib.rs +++ b/litebox_shim_optee/src/lib.rs @@ -22,7 +22,7 @@ use litebox::{ shim::ContinueOperation, utils::TruncateExt, }; -use litebox_common_linux::{errno::Errno, vmap::GlobalVmapManager}; +use litebox_common_linux::{MapFlags, ProtFlags, errno::Errno, vmap::GlobalVmapManager}; use litebox_common_optee::{ LdelfArg, LdelfSyscallRequest, SyscallRequest, TaFlags, TeeAlgorithm, TeeAlgorithmClass, TeeAttributeType, TeeCrypStateHandle, TeeHandleFlag, TeeIdentity, TeeLogin, TeeObjHandle, @@ -269,6 +269,7 @@ impl OpteeShim { ta_entry_point: Cell::new(0), ta_stack_base_addr: Cell::new(0), ta_prepared: Cell::new(false), + stack_guard_page_addr: Cell::new(0), }, }; if let Some(ta_bin) = ta_bin @@ -781,9 +782,12 @@ impl Task { let ta_entry_point = self.get_ta_entry_point(); let mut elf_loader = loader::elf::ElfLoader::new(self, &ta_bin, false)?; elf_loader.load_ta_trampoline(ta_entry_point)?; + self.allocate_stack_guard_page()?; self.ta_prepared.set(true); } + self.restore_stack_guard_fs_base(); + let mut ta_stack = crate::loader::ta_stack::allocate_stack(self, self.get_ta_stack_base_addr()).ok_or( ElfLoaderError::MappingError(litebox::mm::linux::MappingError::OutOfMemory), @@ -792,18 +796,6 @@ impl Task { .init(self.global.platform, params) .ok_or(ElfLoaderError::InvalidStackAddr)?; - // Point the FS base at the TA stack canary so the compiler's - // stack-guard reads resolve to it. Must run after - // `ta_stack.init` (which pushes the canary) and on every entry, - // because the guest FS base does not persist across shim<->guest - // transitions (see `set_guest_stack_guard_fs_base`). - #[cfg(target_arch = "x86_64")] - Self::set_guest_stack_guard_fs_base( - ta_stack - .canary_addr() - .ok_or(ElfLoaderError::InvalidStackAddr)?, - )?; - Ok(ThreadInitState::Ta { cmd_id: cmd_id.unwrap_or(0) as usize, params_address: ta_stack.get_params_address(), @@ -843,31 +835,40 @@ impl Task { } } - /// Point the guest FS base at the TA stack canary so the compiler's stack - /// guard read resolves onto it. - /// - /// OP-TEE TAs are single-threaded and have no TLS block, but the x86-64 - /// toolchain still emits stack-protector reads of `%fs:0x28`. To satisfy - /// those reads without allocating a TLS area, we set the FS base to - /// `canary_addr - ABI_STACK_GUARD_FS_OFFSET` (shared by GCC, Clang, glibc, - /// and musl), so the compiler's guard read lands on the TA stack canary. - /// - /// This must be called on every TA entry: the guest FS base is not - /// guaranteed to persist across a shim<->guest transition. Re-establishing - /// it here keeps the shim platform-agnostic. - /// - /// Returns [`ElfLoaderError::InvalidStackAddr`] if `canary_addr` is below - /// `ABI_STACK_GUARD_FS_OFFSET`. - #[cfg(target_arch = "x86_64")] - fn set_guest_stack_guard_fs_base(canary_addr: usize) -> Result<(), ElfLoaderError> { - use litebox::platform::ArchSpecificProvider as _; - let fs_base = canary_addr - .checked_sub(crate::loader::ta_stack::ABI_STACK_GUARD_FS_OFFSET) + /// Allocate and initialize the page backing the x86-64 stack-guard slot. + fn allocate_stack_guard_page(&self) -> Result<(), ElfLoaderError> { + use litebox::platform::CrngProvider as _; + + let page = self + .sys_mmap( + 0, + PAGE_SIZE, + ProtFlags::PROT_READ_WRITE, + MapFlags::MAP_PRIVATE | MapFlags::MAP_ANONYMOUS | MapFlags::MAP_POPULATE, + -1, + 0, + ) + .map_err(|_| { + ElfLoaderError::MappingError(litebox::mm::linux::MappingError::OutOfMemory) + })?; + let mut guard = [0u8; core::mem::size_of::()]; + self.global.platform.fill_bytes_crng(&mut guard); + page.copy_from_slice(loader::ta_stack::ABI_STACK_GUARD_FS_OFFSET, &guard) .ok_or(ElfLoaderError::InvalidStackAddr)?; + self.sys_mprotect(page, PAGE_SIZE, ProtFlags::PROT_READ) + .map_err(ElfLoaderError::ProtectError)?; + self.stack_guard_page_addr.set(page.as_usize()); + Ok(()) + } + + /// Restore the guest FS base so `%fs:0x28` reads the stack guard page. + fn restore_stack_guard_fs_base(&self) { + use litebox::platform::ArchSpecificProvider as _; + let fs_base = self.stack_guard_page_addr.get(); + debug_assert_ne!(fs_base, 0); litebox_platform_multiplex::platform() .set_arch_specific_register(&litebox::platform::ArchSpecificRegister::FsBase, fs_base) .expect("requires guaranteed platform support for FsBase"); - Ok(()) } /// Retrieve the result of the `ldelf` execution. @@ -1346,6 +1347,8 @@ struct Task { ta_stack_base_addr: Cell, /// Whether the TA has been prepared ta_prepared: Cell, + /// Base address of the read-only page containing the stack guard. + stack_guard_page_addr: Cell, // TODO: OP-TEE supports global, persistent objects across sessions. Add these maps if needed. } @@ -1510,6 +1513,7 @@ mod test_utils { ta_entry_point: Cell::new(0), ta_stack_base_addr: Cell::new(0), ta_prepared: Cell::new(false), + stack_guard_page_addr: Cell::new(0), } } } diff --git a/litebox_shim_optee/src/loader/ta_stack.rs b/litebox_shim_optee/src/loader/ta_stack.rs index 287178240..00d8730d6 100644 --- a/litebox_shim_optee/src/loader/ta_stack.rs +++ b/litebox_shim_optee/src/loader/ta_stack.rs @@ -24,9 +24,8 @@ use crate::{Platform, UserMutPtr}; /// The guard itself is a single pointer-sized word, i.e., **8 bytes** on x86-64. /// The read therefore spans `[%fs:0x28 .. %fs:0x30)`. /// -/// OP-TEE TAs have no TLS block, so the shim sets the guest FS base to -/// `canary_addr - ABI_STACK_GUARD_FS_OFFSET`, making that read resolve onto the -/// CRNG canary pushed by [`TaStack::init`]. +/// OP-TEE TAs have no TLS block, so the shim provides a dedicated stack-guard +/// page whose guard word is stored at this offset. #[cfg(target_arch = "x86_64")] pub(crate) const ABI_STACK_GUARD_FS_OFFSET: usize = 0x28; @@ -126,6 +125,7 @@ impl TaStack { /// Get the address of the TA stack canary pushed by [`Self::init`]. /// Returns `None` if no canary has been pushed yet. #[cfg(target_arch = "x86_64")] + #[expect(dead_code, reason = "retained with the existing stack canary")] pub(crate) fn canary_addr(&self) -> Option { self.canary_pos.map(|pos| self.stack_top.as_usize() + pos) } diff --git a/litebox_shim_optee/src/syscalls/tests.rs b/litebox_shim_optee/src/syscalls/tests.rs index 441228918..0908b60ba 100644 --- a/litebox_shim_optee/src/syscalls/tests.rs +++ b/litebox_shim_optee/src/syscalls/tests.rs @@ -42,6 +42,24 @@ fn test_cryp_random_number_generate() { assert!(result.is_ok() && buf != [0u8; 16]); } +#[test] +fn test_stack_guard_page_is_initialized() { + use litebox::platform::RawConstPointer as _; + + let task = init_platform(); + task.allocate_stack_guard_page().unwrap(); + + let base = task.stack_guard_page_addr.get(); + assert_ne!(base, 0); + assert_eq!(base % litebox::mm::linux::PAGE_SIZE, 0); + let guard = crate::UserConstPtr::::from_usize( + base + crate::loader::ta_stack::ABI_STACK_GUARD_FS_OFFSET, + ) + .read_at_offset(0) + .unwrap(); + assert_ne!(guard, 0); +} + #[test] fn test_sys_get_time_system_is_monotonic() { use litebox::platform::RawConstPointer as _; From 3ac4e34f287c4e88e876e99d69efcc8daa28265c Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Wed, 15 Jul 2026 20:08:01 +0000 Subject: [PATCH 5/5] stack guard page --- litebox_shim_optee/src/lib.rs | 11 +++++++++++ litebox_shim_optee/src/loader/ta_stack.rs | 24 +---------------------- litebox_shim_optee/src/syscalls/tests.rs | 18 ----------------- 3 files changed, 12 insertions(+), 41 deletions(-) diff --git a/litebox_shim_optee/src/lib.rs b/litebox_shim_optee/src/lib.rs index be55df954..ff5af8e08 100644 --- a/litebox_shim_optee/src/lib.rs +++ b/litebox_shim_optee/src/lib.rs @@ -269,6 +269,7 @@ impl OpteeShim { ta_entry_point: Cell::new(0), ta_stack_base_addr: Cell::new(0), ta_prepared: Cell::new(false), + #[cfg(target_arch = "x86_64")] stack_guard_page_addr: Cell::new(0), }, }; @@ -836,6 +837,11 @@ impl Task { } /// Allocate and initialize the page backing the x86-64 stack-guard slot. + /// + /// The x86-64 toolchain emits stack-protector accesses to `%fs:0x28`. + /// Normally glibc or musl initializes that ABI slot before application code + /// runs. OP-TEE TAs use neither runtime, so the shim must provide and + /// initialize the slot before entering a protected TA. fn allocate_stack_guard_page(&self) -> Result<(), ElfLoaderError> { use litebox::platform::CrngProvider as _; @@ -853,6 +859,9 @@ impl Task { })?; let mut guard = [0u8; core::mem::size_of::()]; self.global.platform.fill_bytes_crng(&mut guard); + // Terminator-canary convention (matches glibc `_dl_setup_stack_chk_guard`): + // zero the lowest-addressed byte of the guard to stop the overflow by C string func. + guard[0] = 0; page.copy_from_slice(loader::ta_stack::ABI_STACK_GUARD_FS_OFFSET, &guard) .ok_or(ElfLoaderError::InvalidStackAddr)?; self.sys_mprotect(page, PAGE_SIZE, ProtFlags::PROT_READ) @@ -1348,6 +1357,7 @@ struct Task { /// Whether the TA has been prepared ta_prepared: Cell, /// Base address of the read-only page containing the stack guard. + #[cfg(target_arch = "x86_64")] stack_guard_page_addr: Cell, // TODO: OP-TEE supports global, persistent objects across sessions. Add these maps if needed. } @@ -1513,6 +1523,7 @@ mod test_utils { ta_entry_point: Cell::new(0), ta_stack_base_addr: Cell::new(0), ta_prepared: Cell::new(false), + #[cfg(target_arch = "x86_64")] stack_guard_page_addr: Cell::new(0), } } diff --git a/litebox_shim_optee/src/loader/ta_stack.rs b/litebox_shim_optee/src/loader/ta_stack.rs index 00d8730d6..f9f771800 100644 --- a/litebox_shim_optee/src/loader/ta_stack.rs +++ b/litebox_shim_optee/src/loader/ta_stack.rs @@ -79,9 +79,6 @@ pub struct TaStack { num_params: usize, /// Position where LdelfArg was pushed (if any) ldelf_arg_pos: Option, - #[cfg(target_arch = "x86_64")] - /// Position where the TA stack canary was pushed (if any). - canary_pos: Option, } impl TaStack { @@ -104,7 +101,6 @@ impl TaStack { params: UteeParams::new(), num_params: 0, ldelf_arg_pos: None, - canary_pos: None, }) } @@ -122,14 +118,6 @@ impl TaStack { self.stack_top.as_usize() + self.len - core::mem::size_of::() } - /// Get the address of the TA stack canary pushed by [`Self::init`]. - /// Returns `None` if no canary has been pushed yet. - #[cfg(target_arch = "x86_64")] - #[expect(dead_code, reason = "retained with the existing stack canary")] - pub(crate) fn canary_addr(&self) -> Option { - self.canary_pos.map(|pos| self.stack_top.as_usize() + pos) - } - /// Get the address of `LdelfArg` on the stack. /// /// Returns the actual address where `LdelfArg` was pushed via `init_with_ldelf_arg`. @@ -153,16 +141,6 @@ impl TaStack { Some(()) } - /// Push the TA stack canary and record its address. - fn push_canary(&mut self, canary: &[u8; 16]) -> Option<()> { - self.push_bytes(canary)?; - #[cfg(target_arch = "x86_64")] - { - self.canary_pos = Some(self.pos); - } - Some(()) - } - /// Zero the unused stack region before a new session writes its parameters, /// to avoid leaking leftover data from a prior session whose stack region was recycled. /// The trailing `UteeParams` slot is left untouched here because @@ -298,7 +276,7 @@ impl TaStack { // Random 16-byte stack canary let mut canary = [0u8; 16]; ::fill_bytes_crng(platform, &mut canary); - self.push_canary(&canary)?; + self.push_bytes(&canary)?; // `reenter_thread` *jumps* into the TA entry point (which is a function) rather than // calls it. Adjust the stack pointer to ensure post-call stack alignment. diff --git a/litebox_shim_optee/src/syscalls/tests.rs b/litebox_shim_optee/src/syscalls/tests.rs index 0908b60ba..441228918 100644 --- a/litebox_shim_optee/src/syscalls/tests.rs +++ b/litebox_shim_optee/src/syscalls/tests.rs @@ -42,24 +42,6 @@ fn test_cryp_random_number_generate() { assert!(result.is_ok() && buf != [0u8; 16]); } -#[test] -fn test_stack_guard_page_is_initialized() { - use litebox::platform::RawConstPointer as _; - - let task = init_platform(); - task.allocate_stack_guard_page().unwrap(); - - let base = task.stack_guard_page_addr.get(); - assert_ne!(base, 0); - assert_eq!(base % litebox::mm::linux::PAGE_SIZE, 0); - let guard = crate::UserConstPtr::::from_usize( - base + crate::loader::ta_stack::ABI_STACK_GUARD_FS_OFFSET, - ) - .read_at_offset(0) - .unwrap(); - assert_ne!(guard, 0); -} - #[test] fn test_sys_get_time_system_is_monotonic() { use litebox::platform::RawConstPointer as _;