Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linker section alignment isn't suitable for MPU #524

Open
BigPeteB opened this issue Apr 4, 2024 · 1 comment
Open

Linker section alignment isn't suitable for MPU #524

BigPeteB opened this issue Apr 4, 2024 · 1 comment

Comments

@BigPeteB
Copy link

BigPeteB commented Apr 4, 2024

Correct me if I've gotten anything wrong—I'm not a general Cortex-M expert—but as I understand it, MPU regions must be 32-byte aligned. The linker script in cortex-m-rt uses 32-byte alignment only for .gnu.sgstubs to satisfy the SAU, but uses smaller alignment for all other sections.

The potential results could be very harmful, depending on how the MPU regions are configured. Consider the case of .text followed immediately by .rodata or .data:

  1. If we round towards lower addresses when configuring MPU regions, a few instructions of code would end up in the data region and be marked non-executable. This would be disastrous as it would crash.
  2. If we round towards higher addresses, a few bytes of data would end up in the code region and be marked executable. This could potentially be exploited if an attacker could find a way to manipulate that data so that it contains the instructions they desire (or they get lucky and the data already happens to correspond to instructions they can exploit) and then find a way to cause those instructions to be executed.

(I should mention, I'm considering cases that may be a bit beyond what cortex-m-rt supports out of the box, such as the FLASH memory region actually being in RAM, in which case it's not inherently read-only. But even if FLASH corresponds to read-only storage as cortex-m-rt expects, there are still potential crashes or security exploits waiting to happen.)

I think the linker script should be modified to use 32-byte alignment for regions that users would expect to configure separate MPU regions for. Given the current order of sections that are output, this affects almost all of them: 😭

  • .vector_table (read-only, NX)
  • .text (read-only, executable)
  • .rodata (read-only, NX)
  • .data (the destination would be writeable, NX, but the source data in FLASH would be read-only, NX... or more likely would be unmapped since it's not needed after runtime initialization)
  • .gnu.sgstubs (read-only, NX)
  • .bss, .uninit, heap, and stack are probably unaffected since they would likely be in a contiguous MPU region with .data, although maybe some users want to put the heap and stack in their own MPU regions so their sizes can be dynamically adjusted.

Rearranging the linker sections could allow some regions to be consolidated and use fewer MPU regions, which would reduce the need to align/pad some of the sections. (Also, I just noticed that .gnu.sgstubs is placed after the comment /* ## Sections in RAM */, which is incorrect: it's placed in FLASH.)

@adamgreig
Copy link
Member

MPU regions must be 32-byte aligned.

They must be aligned to the size of the region, which must be a power-of-two number of bytes and at least 32 bytes. So, the minimum alignment is 32 bytes, but in practice most sections will have much larger alignment requirements. I think this makes it impractical to adjust the default linker script to align everything; instead, someone wanting to split up the default sections like this will probably need to use a custom linker script with whatever specific alignments they need.

We do easily support custom linker scripts at least, but the documentation around that could be improved. I've made it clearer in #525.

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

No branches or pull requests

2 participants