Skip to content
This repository has been archived by the owner on Nov 28, 2023. It is now read-only.

RAM init code violates pointer provenance and aliasing rules #69

Closed
jonas-schievink opened this issue Nov 24, 2020 · 1 comment · Fixed by #123
Closed

RAM init code violates pointer provenance and aliasing rules #69

jonas-schievink opened this issue Nov 24, 2020 · 1 comment · Fixed by #123

Comments

@jonas-schievink
Copy link

RAM initialization currently passes pointers to these statics:

riscv-rt/src/lib.rs

Lines 346 to 355 in 47ece5f

// Boundaries of the .bss section
static mut _ebss: u32;
static mut _sbss: u32;
// Boundaries of the .data section
static mut _edata: u32;
static mut _sdata: u32;
// Initial values of the .data section (stored in Flash)
static _sidata: u32;

...to r0:

riscv-rt/src/lib.rs

Lines 381 to 382 in 47ece5f

r0::zero_bss(&mut _sbss, &mut _ebss);
r0::init_data(&mut _sdata, &mut _edata, &_sidata);

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)

@romancardenas
Copy link
Contributor

This should be fixed with #123. Unless someone raises any additional concerns, I'll close this issue as soon as #123 gets merged

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

Successfully merging a pull request may close this issue.

2 participants