summaryrefslogtreecommitdiff
path: root/src/include/cpu
diff options
context:
space:
mode:
authorRaul E Rangel <rrangel@chromium.org>2018-06-25 14:22:27 -0600
committerPatrick Georgi <pgeorgi@google.com>2018-06-28 09:01:02 +0000
commiteb5d76a510d7a4b46e6b33d8a697a30510a0a6d7 (patch)
tree50cde77d90a3f5668fb57aff13a687fb3581c112 /src/include/cpu
parentf78f97e156f3ec71c9ec62cac4cf8954728cddf8 (diff)
downloadcoreboot-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.h4
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. */