rust-embedded / cortex-m

Low level access to Cortex-M processors
Apache License 2.0
787 stars 142 forks source link

`cortex_m::singleton` attribute forwarding is unsound in some cases #538

Open jamesmunns opened 3 weeks ago

jamesmunns commented 3 weeks ago

If you use link_section to put a singleton in an uninit section, the "has been taken" boolean is also in uninit memory at startup. We read this value which is uninitialized.

https://github.com/rust-embedded/cortex-m/blob/f3f85e683f98d0f44ddadcb725e6e05c51c978b1/cortex-m/src/macros.rs#L72-L74

We could potentially put the bool in a separate static that is in the "normal" .bss section, but we'd have to document this.

CC @jordens and https://github.com/rust-embedded/cortex-m/pull/522

jordens commented 3 weeks ago

And it's especially bad for a bool being the prime example of not just uninit but even init with the wrong value causing UB.

jamesmunns commented 3 weeks ago

(fwiw, "link_section" is always an easy way to cause unsoundness in safe code, as with basically any direction you give to the linker, but it FEELS like this is an unexpected outcome rather than an intended one, which we should probably fix and document, or at least document if we say "you can totally use this with link_section".)