rust-lang / rust · Issue No. 160603
I was looking at this SGX code: https://github.com/rust-lang/rust/blob/0312931d8c0ba1a28268a12c06202b68cbc65f76/library/std/src/sys/pal/sgx/waitqueue/unsafe_list.rs
It's a linked-list implementation that freely mixes pointers and references, which is a recipe for disastrous UB. Indeed, simply pushing and popping an element executes UB, as detected by Miri even with Tree Borrows.
Here's a playground based on the SGX code which executes UB: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=2655d6852f717956f2555a6688dd86c2
Since remove takes an exclusive/mutable reference, the fallback path of WaitQueue::wait_timeout can execute UB as far as I can tell: https://github.com/rust-lang/rust/blob/0312931d8c0ba1a28268a12c06202b68cbc65f76/library/std/src/sys/pal/sgx/waitqueue/mod.rs#L180
(To be precise, creating the new mutable reference and passing it into a function invalidates any pointers to entry already in the list. If entry was the first entry, that includes self.head_tail. Then, assert!(!self.is_empty()) executes UB. Maybe I need to add an "unless T: !Unpin" carve-out, not sure.)
Two things: avoid taking exclusive/mutable references as inputs to the UnsafeList, and exclusively use raw pointers in the linked list.
SGX's waitqueue code pushes internally mutable types into the list, so taking mutable references to entries should be unnecessary. Shared references should work fine.
Instead of creating mutable references for the sake of doing writes... just write using the raw pointers. Same for reads, don't create temporary references. Reference->pointer conversion should happen as soon as possible after taking an input, and pointer->reference conversion should happen as late possible just before returning an output.
Relay reads this issue against the repository's contribution signals: the files it is likely to touch, how the maintainers triage work this size, and what the first contribution would exercise.
The full analysis for this issue is still being assembled. Until then, the description above and the thread on GitHub are the most reliable context.