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

ARM_MPU_REGION_MIN_ALIGN_AND_SIZE has invalid value when FPU_SHARING && MPU_STACK_GUARD #83714

Closed
rysiof opened this issue Jan 8, 2025 · 4 comments · Fixed by #83716
Closed
Assignees
Labels
area: ARM ARM (32-bit) Architecture bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@rysiof
Copy link
Contributor

rysiof commented Jan 8, 2025

Describe the bug
With our builds for Arm M7 we are selecting FPU_SHARING and MPU_STACK_GUARD. For that setup, we have ARM_MPU_REGION_MIN_ALIGN_AND_SIZE=64 and MPU_STACK_GUARD_MIN_SIZE_FLOAT=128. So worst case scenario, we are going to have stack that is 64-byte aligned and 128-byte guard at the bottom of the buffer.

https://developer.arm.com/documentation/101407/0541/Debugging/Debug-Windows-and-Dialogs/Core-Peripherals/Armv7-M-cores/Armv7-M--Memory-Protection-Unit tells us that The base address is aligned to the size of the region.

Therefore ARM_MPU_REGION_MIN_ALIGN_AND_SIZE must be 128 when FPU_SHARING && MPU_STACK_GUARD

To Reproduce

Expected behavior

Impact
security and debugability

Logs and console output

Environment (please complete the following information):
All

Additional context

@rysiof rysiof added the bug The issue is a bug, or the PR is fixing a bug label Jan 8, 2025
rysiof added a commit to rysiof/zephyr that referenced this issue Jan 8, 2025
With our builds for Arm M7 we are selecting FPU_SHARING and
MPU_STACK_GUARD. For that setup, we have
ARM_MPU_REGION_MIN_ALIGN_AND_SIZE=64 and
MPU_STACK_GUARD_MIN_SIZE_FLOAT=128. So worst case scenario,
we are going to have stack that is 64-byte aligned and
128-byte guard at the bottom of the buffer.

Fixes: zephyrproject-rtos#83714

Signed-off-by: Maciej Kusio <[email protected]>
@henrikbrixandersen henrikbrixandersen added the area: ARM ARM (32-bit) Architecture label Jan 9, 2025
@kartben kartben assigned wearyzen and unassigned ithinuel Jan 14, 2025
@kartben kartben added the priority: low Low impact/importance bug label Jan 14, 2025
@wearyzen
Copy link
Collaborator

Hi @rysiof ,

So worst case scenario, we are going to have stack that is 64-byte aligned and 128-byte guard at the bottom of the buffer.

Z_MPU_GUARD_ALIGN is max of MPU_GUARD_ALIGN_AND_SIZE and MPU_GUARD_ALIGN_AND_SIZE_FLOAT while,
ARCH_THREAD_STACK_OBJ_ALIGN is max of Z_THREAD_MIN_STACK_ALIGN and Z_MPU_GUARD_ALIGN. Please correct me if I am missing something but FWIU stack cannot be smaller than guard according to this.
Do you have any test scenario and steps to confirm that stack was actually smaller than the guard?

Thanks!

@rysiof
Copy link
Contributor Author

rysiof commented Jan 20, 2025

Hi @wearyzen
Issue is a bit different. Here is an example:
data: 0x10000000 - 0x10000480
stack1: 0x10000480 - 0x10001080

When we switch to stack1, zephyr will try to program MPU guard to
address=0x10000480 and size=128 (due to FPU_SHARING) in my case. This won't work properly as MPU address and size must be aligned. The way MPU will behave is that it will align down address (confirmed with Arm Fast Models) and we will protect address=0x10000400 and size=128 (so not as expected).

So top part of data won't be accessible, when executing thread on stack1, causing MPU fault exception.

@wearyzen
Copy link
Collaborator

Ok so the issue could be because of this or this.

In that case the PR looks good.

@rysiof
Copy link
Contributor Author

rysiof commented Jan 20, 2025

Tyty! I really appreciate that!

robi251 pushed a commit to bobi251/eth_energy_efficiency that referenced this issue Jan 22, 2025
With our builds for Arm M7 we are selecting FPU_SHARING and
MPU_STACK_GUARD. For that setup, we have
ARM_MPU_REGION_MIN_ALIGN_AND_SIZE=64 and
MPU_STACK_GUARD_MIN_SIZE_FLOAT=128. So worst case scenario,
we are going to have stack that is 64-byte aligned and
128-byte guard at the bottom of the buffer.

Fixes: zephyrproject-rtos#83714

Signed-off-by: Maciej Kusio <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM ARM (32-bit) Architecture bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants