Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

RAM initialization code violates pointer provenance #300

Closed
jonas-schievink opened this issue Nov 19, 2020 · 4 comments · Fixed by #301
Closed

RAM initialization code violates pointer provenance #300

jonas-schievink opened this issue Nov 19, 2020 · 4 comments · Fixed by #301
Labels

Comments

@jonas-schievink
Copy link
Contributor

cortex-m-rt/src/lib.rs

Lines 939 to 959 in 96525a6

extern "C" {
// These symbols come from `link.x`
static mut __sbss: u32;
static mut __ebss: u32;
static mut __sdata: u32;
static mut __edata: u32;
static __sidata: u32;
}
extern "Rust" {
// This symbol will be provided by the user via `#[pre_init]`
fn __pre_init();
}
__pre_init();
// Initialize RAM
r0::zero_bss(&mut __sbss, &mut __ebss);
r0::init_data(&mut __sdata, &mut __edata, &__sidata);

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
Copy link
Member

jamesmunns commented Nov 25, 2020

@jonas-schievink
Copy link
Contributor Author

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:

  • Add compiler_fences to r0 (done in Insert a compiler_fence after initialization r0#25), publish that as 1.0.1. We might want to yank 1.0.0, but we won't yank/deprecate the whole r0 crate.
  • Land Initialize RAM in assembly #301 anyways, because it results in better assembly than what LLVM generates.
  • Leave it to the maintainers of downstream -rt crates whether they think they need to do the initialization in assembly.

bors bot added a commit to rust-embedded/r0 that referenced this issue Nov 27, 2020
25: Insert a `compiler_fence` after initialization r=therealprof a=jonas-schievink

cc rust-embedded/cortex-m-rt#300

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
@RalfJung
Copy link

RalfJung commented Nov 28, 2020

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: rust-lang/unsafe-code-guidelines#259.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants