From 8153144de0d27fff686647c4d2b2baf08fa462ee Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Thu, 6 Feb 2020 12:58:53 -0800 Subject: [PATCH] kernel/fatal: Fatal errors must not be preempted The code underneath z_fatal_error() (which is usually run in an exception context, but is not required to be) was running with interrupts enabled, which is a little surprising. The only bug present currently is that the CPU ID extracted for logging is subject to a race (i.e. it's possible but very unlikely that such a handler might migrate to another CPU after the error is flagged and log the wrong CPU ID), but in general users with custom error handlers are likely to be surprised when their dying threads gets preempted by other code before they can abort. Signed-off-by: Andy Ross --- kernel/fatal.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/kernel/fatal.c b/kernel/fatal.c index cb6441abf30..eae7ff9f6cf 100644 --- a/kernel/fatal.c +++ b/kernel/fatal.c @@ -92,6 +92,11 @@ static inline int get_cpu(void) void z_fatal_error(unsigned int reason, const z_arch_esf_t *esf) { + /* We can't allow this code to be preempted, but don't need to + * synchronize between CPUs, so an arch-layer lock is + * appropriate. + */ + unsigned int key = arch_irq_lock(); struct k_thread *thread = k_current_get(); /* sanitycheck looks for the "ZEPHYR FATAL ERROR" string, don't @@ -160,5 +165,6 @@ void z_fatal_error(unsigned int reason, const z_arch_esf_t *esf) #endif /*CONFIG_ARCH_HAS_NESTED_EXCEPTION_DETECTION */ } + arch_irq_unlock(key); k_thread_abort(thread); }