ARM: MPU-based HW thread stack protection not working properly when building with CONFIG_FLOAT

[ioannisg]

In ARM architecture, MPU-based stack protection works, briefly, as follows:

• a Guard area at the bottom of supervisor thread is given Read-Only access
• thread stack overflow eventually attempts to push into the guard, triggering an MPU fault (Data Access Violation)
• the MPU fault triggers the MemManage exception, which leads to, a synchronous MPU stacking error, since the MemManage exception attempts to stack an exception stack frame.
  When building without FLOAT, the length of the exception frame is 8 words, which matches exactly the length of the guard. Therefore, no stack corruption in the area beyond the stack can occur due to a thread stack overflow.

However, when building with CONFIG_FLOAT the FP context may be active at the time of a stack overflow. The MemManage exception will attempt to stack an "extended" exception frame that includes the FP-context (18 additional words).

In that scenario we still get a MemManage Stacking Error (Note: when Lazy-Stacking is disabled, which, currently, is the default) but the area beyond the end of the guard area is corrupted!

Therefore, we need larger Guard areas when building with FLOAT to prevent stack corruption.

[ioannisg]

FYI @galak @agross-linaro @MaureenHelm

Shall we try to fix this in 1.15?

[andrewboie]

Therefore, we need larger Guard areas when building with FLOAT to prevent stack corruption.

Counter-argument: the mechanism for stack overflows in supervisor mode does not in any way guarantee that some kind of corruption hasn't happened. If a supervisor mode thread blows its stack, whatever kernel data structures it was working on could be in an inconsistent state that is impossible to recover from. We are only trying to ensure that this situation is detected, but actually preventing corruption is out of scope. I don't think this is a bug per se.

This is opposed to stack overflow handling in user mode; in that context, we know that the overall kernel is still in good shape, although whatever application logic that thread was involved in may be hosed.

[ioannisg]

We are only trying to ensure that this situation is detected, but actually preventing corruption is out of scope. I don't think this is a bug per se.

That's correct...for now! When we enable lazy stacking we will need larger guards, even for detecting (not for preventing) corruption.

This is opposed to stack overflow handling in user mode; in that context, we know that the overall kernel is still in good shape, although whatever application logic that thread was involved in may be hosed.

I agree. But could we then start thinking of how to prevent non-essential supervisor threads from not corrupting the kernel?

[andrewboie]

That's correct...for now! When we enable lazy stacking we will need larger guards, even for detecting (not for preventing) corruption.

I'm not sure I understand. Won't the fault happen when the lazy stacking takes place, regardless of the guard size?

But could we then start thinking of how to prevent non-essential supervisor threads from not corrupting the kernel?

I don't think that's possible if they operate on shared data structures. Like wait queues, any kind of concurrency control, etc. I think it's probably simpler to look into what additional subsystems we can push into userspace.

[ioannisg]

I'm not sure I understand. Won't the fault happen when the lazy stacking takes place, regardless of the guard size?

@andrewboie , no.
When Lazy Stacking is enabled, the stack area for the FP context is reserved, but the FP registers are not pushed onto the stack, so no MPU violation occurs. The normal stack frame will be pushed after the FP registers, so with a 32-byte thin guard, we will descend well below the guard area when pushing the standard registers.

During the next context switch the FP context will finally be pushed onto the stack, and this will give an MPU faullt which will be very confusing at that time.

Therefore, lazy stacking really requires a wide guard.

[andrewboie]

The normal stack frame will be pushed after the FP registers, so with a 32-byte thin guard, we will descend well below the guard area when pushing the standard registers.

Got it, thanks for the details!

I think we want to fix this, but perhaps Low priority should be more appropriate.

[andrewboie]

Also if this is to be fixed for 1.14, I think a small non-invasive change should be made, such as setting MPU_GUARD_ALIGN_AND_SIZE to the larger value if CONFIG_FLOAT is turned on.