rust-embedded / cortex-m-rt

Minimal startup / runtime for Cortex-M microcontrollers
https://rust-embedded.github.io/cortex-m-rt/
Apache License 2.0
359 stars 85 forks source link

RAM initialization code violates pointer provenance #300

Closed jonas-schievink closed 3 years ago

jonas-schievink commented 4 years ago

https://github.com/rust-embedded/cortex-m-rt/blob/96525a64197049d11cfc8cb5cc2c4dc9b5240e42/src/lib.rs#L939-L959

This code uses a pointer to a single u32, but writes an arbitrary amount of data beyond the u32. This is UB, as it violates the contract of ptr::offset, which is called by r0. Namely:

Both the starting and resulting pointer must be either in bounds or one byte past the end of the same allocated object. Note that in Rust, every (stack-allocated) variable is considered a separate allocated object.

jamesmunns commented 4 years ago

Linking relevant issues so others can follow breadcrumbs:

We also need to reach out to tock-os, I'll open an issue there shortly.

Edit: Added tock-os issue

nagisa commented 4 years ago

Note relevant discussion: https://rust-lang.zulipchat.com/#narrow/stream/136281-t-lang.2Fwg-unsafe-code-guidelines/topic/The.20least.20incorrect.20init.20code.20.3A)

jonas-schievink commented 4 years ago

Copying @RalfJung's comment from that Zulip thread:

This question is indeed well outside the usual abstract machine model, so I find it hard to say much. It quite clearly violates the aliasing rules, but also in some sense it happens before the abstract machine is even initialized and the aliasing rules start making sense. A compiler barrier is strongly advised, is unfortunately all I can say -- sorry.

I'd say the most practical way to continue is thus:

RalfJung commented 4 years ago

Note that I was talking about the "writing to statics without actually going through the static" part in what you quote above.

The issue about out-of-bounds accesses is a separate one, for which I created a new UCG issue as this has come up before: https://github.com/rust-lang/unsafe-code-guidelines/issues/259.