search

iqlusioninc / usbarmory.rs

Bare metal Rust support for USB armory MkII devices
Apache License 2.0
58 stars 4 forks source link

RAM init code violates pointer provenance and aliasing rules #74

Open jonas-schievink opened 3 years ago

jonas-schievink commented 3 years ago

RAM initialization currently passes pointers to these statics to r0:

https://github.com/iqlusioninc/usbarmory.rs/blob/692433d14609cdd5dfe3f3a89dbc9118a8c7398d/firmware/usbarmory-rt/src/lib.rs#L57-L66

But since r0 will fill the entire memory range between the pointers, it ends up calling offset with values that create pointers well beyond the u32 the static is declared to contain. According to the docs of offset, this is UB:

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.

(every static also counts as its own "allocated object", I believe)

Moreover, the ptr::write and ptr::write_volatile calls in r0 also cause UB, because this invariant is violated:

dst must be valid for writes.

Where the validity of a pointer requires this:

For a pointer to be valid, it is necessary, but not always sufficient, that the pointer be dereferenceable: the memory range of the given size starting at the pointer must all be within the bounds of a single allocated object. Note that in Rust, every (stack-allocated) variable is considered a separate allocated object.

...but again, we are crossing between potentially many allocated objects (every static in the .data/.bss section).

Furthermore, writing to a static through a pointer not derived from that static might violate aliasing rules, but it isn't yet clear if this is the case today (I've asked the folks working on the unsafe code guidelines for clarification / guidance).

The most robust solution for this issue is to write the RAM init code in assembly instead of Rust, and run no Rust code at all until RAM is initialized.

(this issue was recently discovered to affect most -rt crates in the ecosystem; we do not believe it to cause practical issues at the moment)

tony-iqlusion commented 3 years ago

@jonas-schievink thank you for bringing this to our attention. We will look into ASM solutions for RAM init code.