Skip to content

std::sys::sgx::tls: fix TLS destructor pointer provenance - #161960

Open
phlip9 wants to merge 1 commit into
rust-lang:mainfrom
phlip9:phlip9/sgx-fix-tls-ptr-provenance
Open

std::sys::sgx::tls: fix TLS destructor pointer provenance#161960
phlip9 wants to merge 1 commit into
rust-lang:mainfrom
phlip9:phlip9/sgx-fix-tls-ptr-provenance

Conversation

@phlip9

@phlip9 phlip9 commented Aug 29, 2026

Copy link
Copy Markdown

Storing the TLS entry destructor as an AtomicUsize loses the pointer provenance and makes miri cry :'(

Instead, store an AtomicPtr<()>. I also find the SAFETY proof more convincing when the transmute from *mut () to Option<unsafe extern "C" fn(*mut u8)> is exactly mirrored.

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=c6da201f269c5f3115039177a43bcdd4

   Compiling playground v0.0.1 (/playground)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.70s
     Running `/playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/debug/playground`
error: Undefined Behavior: pointer not dereferenceable: pointer must point to some allocation, but got 0x26b51[noalloc] which is a dangling pointer (it has no provenance)
  --> src/tls-bad.rs:73:30
   |
73 |                     unsafe { dtor(value) }
   |                              ^^^^^^^^^^^ Undefined Behavior occurred here
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: stack backtrace:
           0: <tls::ActiveTls<'_> as std::ops::Drop>::drop
               at src/tls-bad.rs:73:30: 73:41
           1: std::ptr::drop_glue::<tls::ActiveTls<'_>> - shim(Some(tls::ActiveTls<'_>))
               at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:848:1: 850:25
           2: main
               at src/main.rs:50:1: 50:2

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

Storing the TLS entry destructor as an `AtomicUsize` loses the pointer
provenance and makes miri cry :'(
@rustbot rustbot added O-SGX Target: SGX S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 29, 2026
@rustbot

rustbot commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome! The Rust Project has assigned @clarfonthey (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks.

Please see the contribution instructions and our LLM policy for more information.

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @ChrisDenton, libs
  • @ChrisDenton, libs expanded to 13 candidates
  • Random selection from ChrisDenton, JohnTitor, Mark-Simulacrum, clarfonthey, nia-e

@maxdexh

maxdexh commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Wow, i recently changed something surrounding this code and i just assumed this was an AtomicPtr, because the alternative makes no sense!

@rustbot label T-opsem

@rustbot rustbot added the T-opsem Relevant to the opsem team label Aug 29, 2026
@clarfonthey

Copy link
Copy Markdown
Contributor

Seems reasonable to me.

@bors try jobs=dist-various-2

(since this job checks x86_64-fortanix-unknown-sgx)

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Aug 29, 2026
…try>

std::sys::sgx::tls: fix TLS destructor pointer provenance


try-job: dist-various-2
@rust-bors

rust-bors Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 20692fd (20692fdf9e77d0edd88cab7130074a6576c0ab02)
Base parent: 6bb812b (6bb812b714cce8e08864b7b569303f25ab08eaad)

@clarfonthey

Copy link
Copy Markdown
Contributor

@bors r+ rollup

Thank you!

@rust-bors

rust-bors Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

📌 Commit a132402 has been approved by clarfonthey

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 29, 2026
…enance, r=clarfonthey

std::sys::sgx::tls: fix TLS destructor pointer provenance

Storing the TLS entry destructor as an `AtomicUsize` loses the pointer provenance and makes miri cry :'(

Instead, store an `AtomicPtr<()>`. I also find the SAFETY proof more convincing when the transmute from `*mut ()` to `Option<unsafe extern "C" fn(*mut u8)>` is exactly mirrored.

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=c6da201f269c5f3115039177a43bcdd4

```
   Compiling playground v0.0.1 (/playground)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.70s
     Running `/playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/debug/playground`
error: Undefined Behavior: pointer not dereferenceable: pointer must point to some allocation, but got 0x26b51[noalloc] which is a dangling pointer (it has no provenance)
  --> src/tls-bad.rs:73:30
   |
73 |                     unsafe { dtor(value) }
   |                              ^^^^^^^^^^^ Undefined Behavior occurred here
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: stack backtrace:
           0: <tls::ActiveTls<'_> as std::ops::Drop>::drop
               at src/tls-bad.rs:73:30: 73:41
           1: std::ptr::drop_glue::<tls::ActiveTls<'_>> - shim(Some(tls::ActiveTls<'_>))
               at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:848:1: 850:25
           2: main
               at src/main.rs:50:1: 50:2

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error
```
rust-bors Bot pushed a commit that referenced this pull request Aug 29, 2026
…uwer

Rollup of 3 pull requests

Successful merges:

 - #160851 (Add MSA and `f16` inline ASM support for MIPS)
 - #161960 (std::sys::sgx::tls: fix TLS destructor pointer provenance)
 - #161976 (Improve rustdoc macro expansion code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

O-SGX Target: SGX S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants