Skip to content

Commit 3ac4e34

Browse files
author
Sangho Lee
committed
stack guard page
1 parent bd67673 commit 3ac4e34

3 files changed

Lines changed: 12 additions & 41 deletions

File tree

‎litebox_shim_optee/src/lib.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ impl OpteeShim {
269269
ta_entry_point: Cell::new(0),
270270
ta_stack_base_addr: Cell::new(0),
271271
ta_prepared: Cell::new(false),
272+
#[cfg(target_arch = "x86_64")]
272273
stack_guard_page_addr: Cell::new(0),
273274
},
274275
};
@@ -836,6 +837,11 @@ impl Task {
836837
}
837838

838839
/// Allocate and initialize the page backing the x86-64 stack-guard slot.
840+
///
841+
/// The x86-64 toolchain emits stack-protector accesses to `%fs:0x28`.
842+
/// Normally glibc or musl initializes that ABI slot before application code
843+
/// runs. OP-TEE TAs use neither runtime, so the shim must provide and
844+
/// initialize the slot before entering a protected TA.
839845
fn allocate_stack_guard_page(&self) -> Result<(), ElfLoaderError> {
840846
use litebox::platform::CrngProvider as _;
841847

@@ -853,6 +859,9 @@ impl Task {
853859
})?;
854860
let mut guard = [0u8; core::mem::size_of::<usize>()];
855861
self.global.platform.fill_bytes_crng(&mut guard);
862+
// Terminator-canary convention (matches glibc `_dl_setup_stack_chk_guard`):
863+
// zero the lowest-addressed byte of the guard to stop the overflow by C string func.
864+
guard[0] = 0;
856865
page.copy_from_slice(loader::ta_stack::ABI_STACK_GUARD_FS_OFFSET, &guard)
857866
.ok_or(ElfLoaderError::InvalidStackAddr)?;
858867
self.sys_mprotect(page, PAGE_SIZE, ProtFlags::PROT_READ)
@@ -1348,6 +1357,7 @@ struct Task {
13481357
/// Whether the TA has been prepared
13491358
ta_prepared: Cell<bool>,
13501359
/// Base address of the read-only page containing the stack guard.
1360+
#[cfg(target_arch = "x86_64")]
13511361
stack_guard_page_addr: Cell<usize>,
13521362
// TODO: OP-TEE supports global, persistent objects across sessions. Add these maps if needed.
13531363
}
@@ -1513,6 +1523,7 @@ mod test_utils {
15131523
ta_entry_point: Cell::new(0),
15141524
ta_stack_base_addr: Cell::new(0),
15151525
ta_prepared: Cell::new(false),
1526+
#[cfg(target_arch = "x86_64")]
15161527
stack_guard_page_addr: Cell::new(0),
15171528
}
15181529
}

‎litebox_shim_optee/src/loader/ta_stack.rs‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,6 @@ pub struct TaStack {
7979
num_params: usize,
8080
/// Position where LdelfArg was pushed (if any)
8181
ldelf_arg_pos: Option<usize>,
82-
#[cfg(target_arch = "x86_64")]
83-
/// Position where the TA stack canary was pushed (if any).
84-
canary_pos: Option<usize>,
8582
}
8683

8784
impl TaStack {
@@ -104,7 +101,6 @@ impl TaStack {
104101
params: UteeParams::new(),
105102
num_params: 0,
106103
ldelf_arg_pos: None,
107-
canary_pos: None,
108104
})
109105
}
110106

@@ -122,14 +118,6 @@ impl TaStack {
122118
self.stack_top.as_usize() + self.len - core::mem::size_of::<UteeParams>()
123119
}
124120

125-
/// Get the address of the TA stack canary pushed by [`Self::init`].
126-
/// Returns `None` if no canary has been pushed yet.
127-
#[cfg(target_arch = "x86_64")]
128-
#[expect(dead_code, reason = "retained with the existing stack canary")]
129-
pub(crate) fn canary_addr(&self) -> Option<usize> {
130-
self.canary_pos.map(|pos| self.stack_top.as_usize() + pos)
131-
}
132-
133121
/// Get the address of `LdelfArg` on the stack.
134122
///
135123
/// Returns the actual address where `LdelfArg` was pushed via `init_with_ldelf_arg`.
@@ -153,16 +141,6 @@ impl TaStack {
153141
Some(())
154142
}
155143

156-
/// Push the TA stack canary and record its address.
157-
fn push_canary(&mut self, canary: &[u8; 16]) -> Option<()> {
158-
self.push_bytes(canary)?;
159-
#[cfg(target_arch = "x86_64")]
160-
{
161-
self.canary_pos = Some(self.pos);
162-
}
163-
Some(())
164-
}
165-
166144
/// Zero the unused stack region before a new session writes its parameters,
167145
/// to avoid leaking leftover data from a prior session whose stack region was recycled.
168146
/// The trailing `UteeParams` slot is left untouched here because
@@ -298,7 +276,7 @@ impl TaStack {
298276
// Random 16-byte stack canary
299277
let mut canary = [0u8; 16];
300278
<Platform as litebox::platform::CrngProvider>::fill_bytes_crng(platform, &mut canary);
301-
self.push_canary(&canary)?;
279+
self.push_bytes(&canary)?;
302280

303281
// `reenter_thread` *jumps* into the TA entry point (which is a function) rather than
304282
// calls it. Adjust the stack pointer to ensure post-call stack alignment.

‎litebox_shim_optee/src/syscalls/tests.rs‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -42,24 +42,6 @@ fn test_cryp_random_number_generate() {
4242
assert!(result.is_ok() && buf != [0u8; 16]);
4343
}
4444

45-
#[test]
46-
fn test_stack_guard_page_is_initialized() {
47-
use litebox::platform::RawConstPointer as _;
48-
49-
let task = init_platform();
50-
task.allocate_stack_guard_page().unwrap();
51-
52-
let base = task.stack_guard_page_addr.get();
53-
assert_ne!(base, 0);
54-
assert_eq!(base % litebox::mm::linux::PAGE_SIZE, 0);
55-
let guard = crate::UserConstPtr::<usize>::from_usize(
56-
base + crate::loader::ta_stack::ABI_STACK_GUARD_FS_OFFSET,
57-
)
58-
.read_at_offset(0)
59-
.unwrap();
60-
assert_ne!(guard, 0);
61-
}
62-
6345
#[test]
6446
fn test_sys_get_time_system_is_monotonic() {
6547
use litebox::platform::RawConstPointer as _;

0 commit comments

Comments
 (0)