Enable stack overflow protection by default

[jonas-schievink]

Not providing this is unsound, so we should make sure this is on by default before 1.0. This was already implemented in the past, but was removed with the switch to LLD (if I'm not mistaken).

@japaric has been working on flip-lld, a small wrapper that invokes LLD twice to place the stack below the static data sections. An older description of this approach is described in this blog article.

(unfortunately, experiments with doing it with one linker invocation have so far not worked out, but according to rust-embedded/cortex-m-rt#34 (comment) there's another way to do it with a single linker invocation, might be worth checking out)

[therealprof]

@jonas-schievink I do not agree with the "unsound" part of your intro but it would certainly be nice to have it.

[Disasm]

This may not work well on Cortex-M0 microcontrollers when you have to relocate vector table to the start of the RAM. For example, when you have a bootloader at the beginning of the FLASH with its own interrupt vector table and you want to have your own one.

[jonas-schievink]

@therealprof "unsound" just means that it's possible to cause undefined behavior in safe code, which is the case here (just grow the stack enough). While there are exceptions to this that aren't really modeled by Rust's type system (such as writing to /dev/mem, or DMAing all over RAM), creating local variables isn't one of them.

@Disasm Ah, you mean when using a vendor-specific feature to map SRAM to address 0. I'm hoping that we'll add features for controlling the memory layout more precisely before 1.0, and it doesn't seem like this situation would be worse than what we have now (you'd just smash the vector table instead of application data, which is hopefully more noticeable).

[Disasm]

@jonas-schievink

you'd just smash the vector table instead of application data, which is hopefully more noticeable

This sounds like a code execution vulnerability out of the box. Quite easy to exploit.

[jonas-schievink]

That's true, but the same is the case today from what I can tell (only you'd also have to overwrite .data and .bss in RAM). Since the M0 also doesn't have an MPU that could be used to mark the region read/execute-only, it seems like the only real fix would be to use a vendor-specific protection feature for SRAM or the stack (not sure what's offered here), or to add stack checking code to every function call (which needs rustc support).

[Disasm]

Anyway, I think it's a good idea to have this stack protection feature, but I'd also like to have a possibility for opt-out.

[japaric]

For reference / posteriority: the trick I have tried is "end address" aligning
sections in the linker script. The trick looks like this:

SECTIONS
{
  .bss ORIGIN(RAM) + LENGTH(RAM) - SIZEOF(.bss) :
  /*   ^^^^^^^^^^^^^^^^^^^^^^^^^                */
  /*   desired end address                      */
  {
    . = ALIGN(4);
    *(.bss .bss.*);
    . = ALIGN(4);
  } > RAM

  .data ADDR(.bss) - SIZEOF(.data) :
  /*    ^^^^^^^^^^^^^^^^^^^^^^^^^ */
  /*    start address             */
  {
    . = ALIGN(4);
    *(.data .data.*);
    . = ALIGN(4);
  } > RAM

  /* .. */
}

The expression after the section name (.bss) and before the : is the start
address (Virtual Memory Address, to be precise) of the section. The trick is
applying the SIZEOF operator, which returns (or should return) the size of the
section, on the section itself to decide its start address.

A complete, working example can be found here: https://github.com/japaric/xenon/blob/02e36e97777a8ebc53ccf202bb3cf278e1662de9/firmware/xenon/link.x#L25

For whatever reason, that example breaks down (you get weird linker errors) once
the .rodata has a non-zero size. That could be a LLD bug; I don't know. The
example doesn't link when you use GNU LD.

[pwnorbitals]

https://app.bountysource.com/issues/110784472-enable-stack-overflow-protection-by-default