diff options
author | Raul E Rangel <rrangel@chromium.org> | 2018-06-25 14:22:27 -0600 |
---|---|---|
committer | Patrick Georgi <pgeorgi@google.com> | 2018-06-28 09:01:02 +0000 |
commit | eb5d76a510d7a4b46e6b33d8a697a30510a0a6d7 (patch) | |
tree | 50cde77d90a3f5668fb57aff13a687fb3581c112 /src/include/cpu | |
parent | f78f97e156f3ec71c9ec62cac4cf8954728cddf8 (diff) | |
download | coreboot-eb5d76a510d7a4b46e6b33d8a697a30510a0a6d7.tar.xz |
smm: Add canary to end of stack and die() if a stack overflow occurs
If CPU 0's stack grows to large, it will overflow into CPU 1's stack.
If CPU 0 is handling the interrupt then CPU 1 should be in an idle loop.
When the stack overflow occurs it will override the return pointer for
CPU 1, so when CPU 0 unlocks the SMI lock, CPU 1 will attempt to return
to a random address.
This method is not foolproof. If code allocates some stack variables
that overlap with the canary, and if the variables are never set, then
the canary will not be overwritten, but it will have been skipped. We
could mitigate this by adding a larger canary value if we wanted.
I chose to use the stack bottom pointer value as the canary value
because:
* It will change per CPU stack.
* Doesn't require hard coding a value that must be shared between the
.S and .c.
* Passing the expected canary value as a parameter felt like overkill.
We can explore adding other methods of signaling that a stack overflow
had occurred in a follow up. I limited die() to debug only because
otherwise it would be very hard to track down.
TEST=built on grunt with a small and large stack size. Then verified
that one causes a stack overflow and the other does not.
Stack overflow message:
canary 0x0 != 0xcdeafc00
SMM Handler caused a stack overflow
Change-Id: I0184de7e3bfb84e0f74e1fa6a307633541f55612
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Reviewed-on: https://review.coreboot.org/27229
Reviewed-by: Martin Roth <martinroth@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Diffstat (limited to 'src/include/cpu')
-rw-r--r-- | src/include/cpu/x86/smm.h | 4 |
1 files changed, 4 insertions, 0 deletions
diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 7dcf4d70e2..9942772f0e 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -527,6 +527,10 @@ struct smm_module_params { void *arg; int cpu; const struct smm_runtime *runtime; + /* A canary value that has been placed at the end of the stack. + * If (uintptr_t)canary != *canary then a stack overflow has occurred. + */ + const uintptr_t *canary; }; /* smm_handler_t is called with arg of smm_module_params pointer. */ |