Skip to content

Enable x86_64 Linux stack guard for OP-TEE TAs - #1003

Draft
Sangho Lee (sangho2) wants to merge 5 commits into
mainfrom
sanghle/optee/notls
Draft

Enable x86_64 Linux stack guard for OP-TEE TAs#1003
Sangho Lee (sangho2) wants to merge 5 commits into
mainfrom
sanghle/optee/notls

Conversation

@sangho2

@sangho2 Sangho Lee (sangho2) commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

This PR enables stack guard for OP-TEE TAs. Currently, we use x86_64 Linux toolchains to build OP-TEE TA binaries with -fstack-protector (enabled by default), which expects glibc/musl stores a stack canary value in %fs:0x28. However, OP-TEE TAs don't use glibc/musl such that no code prepares memory for this. This PR lets the OP-TEE shim allocate, initialize (with a random number), and protect a stack guard page.

@sangho2
Sangho Lee (sangho2) force-pushed the sanghle/optee/notls branch 2 times, most recently from e3d2fee to 95eeab4 Compare July 2, 2026 23:33
@sangho2
Sangho Lee (sangho2) marked this pull request as ready for review July 2, 2026 23:45
@sangho2
Sangho Lee (sangho2) marked this pull request as draft July 15, 2026 17:01
@sangho2 Sangho Lee (sangho2) changed the title Satisfy the OP-TEE TA stack-guard read via the stack canary Enable x86_64 Linux style stack guard for OP-TEE TAs Jul 15, 2026
@sangho2 Sangho Lee (sangho2) changed the title Enable x86_64 Linux style stack guard for OP-TEE TAs Enable x86_64 Linux stack guard for OP-TEE TAs Jul 15, 2026
@sangho2
Sangho Lee (sangho2) marked this pull request as ready for review July 15, 2026 20:27
@github-actions

Copy link
Copy Markdown

🤖 SemverChecks 🤖 No breaking API changes detected

Note: this does not mean API is unchanged, or even that there are no breaking changes; simply, none of the detections triggered.

@wdcui

Copy link
Copy Markdown
Member

Weiteng Chen (@CvvT) , can you also take a look at this PR? The current approach feels a little hacky to me. It would be good to see if you have any suggestions.

@CvvT Weiteng Chen (CvvT) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the fix is about setting the canary and page permission properly; I don't see why the rest of the change (i.e., renaming stuffs) is necessary. I feel allocate_guest_tls is more accurate than allocate_stack_guard_page (the later one sounds like we allocate a guard page for stack, but the page is for tls; we just initialize the canary in it).

@CvvT

Weiteng Chen (CvvT) commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Weiteng Chen (@CvvT) , can you also take a look at this PR? The current approach feels a little hacky to me. It would be good to see if you have any suggestions.

I don't have a better idea. The hardcoded offset looks fine to me (it's a fixed value per target ABI). If we cannot control how to compile the TAs, I don't think there is other way around it.

@sangho2

Copy link
Copy Markdown
Contributor Author

Looks like the fix is about setting the canary and page permission properly; I don't see why the rest of the change (i.e., renaming stuffs) is necessary. I feel allocate_guest_tls is more accurate than allocate_stack_guard_page (the later one sounds like we allocate a guard page for stack, but the page is for tls; we just initialize the canary in it).

The reference OP-TEE version (which is outdated btw) doesn't use thread pointers at all. That's why I thought the guest TLS is misleading. I've checked the lasted OP-TEE (for Arm) and confirmed that it now has guest TLS just for compiler compatibility (like this PR). In that sense, allocate_guest_tls does make sense. Let me revert this name change.

@sangho2

Copy link
Copy Markdown
Contributor Author

Weiteng Chen (@CvvT) , can you also take a look at this PR? The current approach feels a little hacky to me. It would be good to see if you have any suggestions.

I don't have a better idea. The hardcoded offset looks fine to me (it's a fixed value per target ABI). If we cannot control how to compile the TAs, I don't think there is other way around it.

The latest OP-TEE (for Arm) now does this TLS allocation in user-mode libutee for compiler compatibility (like this PR). Our reference OP-TEE (for x86) is, unfortunately, pretty outdated and thus doesn't have this code. We could get rid of this hack once our toolchain gets updated (no ETA AFAIK).

@sangho2 Sangho Lee (sangho2) added discussion Open questions must-not-merge:undergoing-restructuring Known deeper set of changes are happening on this PR before it is mergeable again labels Jul 16, 2026
@sangho2
Sangho Lee (sangho2) marked this pull request as draft July 16, 2026 22:15
self.restore_guest_tls();
/// 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OP-TEE TAs use neither runtime

Is this specific to TAs running in LiteBox?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a stale/draft PR.

The main issue is that libutee we are using is unaware of x86-gcc/libc's convention, such that it does not prepare FSBase as gcc expects. Revising the devkit (or its build configuration) would be a fundamental solution.

#[cfg(target_arch = "x86_64")]
fn restore_guest_tls(&self) {
/// Restore the guest FS base so `%fs:0x28` reads the stack guard page.
fn restore_stack_guard_fs_base(&self) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: As you are setting FSBase register value here, restore in the function name is a bit confusing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using restore here because user-mode LiteBox and guests share FSBase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discussion Open questions must-not-merge:undergoing-restructuring Known deeper set of changes are happening on this PR before it is mergeable again

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants