From 41576338fb808adc47d24deb6f1e433e535c7ab5 Mon Sep 17 00:00:00 2001 From: Terraneo Federico <fede.tft@miosix.org> Date: Fri, 10 Jan 2025 16:29:31 +0100 Subject: [PATCH] Fix stack overflow check in processes if a syscall immediately followed the overflow --- miosix/arch/cpu/common/cortexMx_interrupts.cpp | 6 +++++- miosix/kernel/kernel.cpp | 18 +++++++++--------- miosix/kernel/kernel.h | 15 ++++++++++++--- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/miosix/arch/cpu/common/cortexMx_interrupts.cpp b/miosix/arch/cpu/common/cortexMx_interrupts.cpp index 113a8a5a..a2514b9e 100644 --- a/miosix/arch/cpu/common/cortexMx_interrupts.cpp +++ b/miosix/arch/cpu/common/cortexMx_interrupts.cpp @@ -618,7 +618,11 @@ void __attribute__((noinline)) svcImpl() return; } } - Thread::IRQstackOverflowCheck(); + // NOTE: Do not process the syscall if a stack overflow was detected, since + // the process will segfault anyway. Moreover, the stack overflow check code + // switches the thread context to the kernelspace one and attempting to + // extract the syscall number from ctxsave will return garbage! + if(Thread::IRQstackOverflowCheck()) return; //Miosix on ARM uses r3 for the syscall number. //Note that it is required to use ctxsave and not cur->ctxsave because diff --git a/miosix/kernel/kernel.cpp b/miosix/kernel/kernel.cpp index 39df313f..0b80bd18 100755 --- a/miosix/kernel/kernel.cpp +++ b/miosix/kernel/kernel.cpp @@ -617,29 +617,29 @@ int Thread::getStackSize() return getCurrentThread()->stacksize; } -void Thread::IRQstackOverflowCheck() +bool Thread::IRQstackOverflowCheck() { const unsigned int watermarkSize=WATERMARK_LEN/sizeof(unsigned int); #ifdef WITH_PROCESSES if(const_cast<Thread*>(runningThread)->flags.isInUserspace()) { bool overflow=false; - for(unsigned int i=0;i<watermarkSize;i++) - if(runningThread->userWatermark[i]!=WATERMARK_FILL) overflow=true; if(runningThread->userCtxsave[stackPtrOffsetInCtxsave] < reinterpret_cast<unsigned int>(runningThread->userWatermark+watermarkSize)) overflow=true; + if(overflow==false) + for(unsigned int i=0;i<watermarkSize;i++) + if(runningThread->userWatermark[i]!=WATERMARK_FILL) overflow=true; if(overflow) IRQreportFault(FaultData(fault::STACKOVERFLOW,0)); - } else { + return overflow; + } #endif //WITH_PROCESSES - for(unsigned int i=0;i<watermarkSize;i++) - if(runningThread->watermark[i]!=WATERMARK_FILL) errorHandler(STACK_OVERFLOW); if(runningThread->ctxsave[stackPtrOffsetInCtxsave] < reinterpret_cast<unsigned int>(runningThread->watermark+watermarkSize)) errorHandler(STACK_OVERFLOW); - #ifdef WITH_PROCESSES - } - #endif //WITH_PROCESSES + for(unsigned int i=0;i<watermarkSize;i++) + if(runningThread->watermark[i]!=WATERMARK_FILL) errorHandler(STACK_OVERFLOW); + return false; } #ifdef WITH_PROCESSES diff --git a/miosix/kernel/kernel.h b/miosix/kernel/kernel.h index 829519bb..5033e5d5 100755 --- a/miosix/kernel/kernel.h +++ b/miosix/kernel/kernel.h @@ -888,10 +888,19 @@ public: /** * \internal - * Used before every context switch to check if the stack of the thread - * being preempted has overflowed + * To be used in interrupts where a context switch can occur to check if the + * stack of the thread being preempted has overflowed. + * Note that since Miosix 3 all peripheral interrupts no longer perform a + * full context save/restore thus you cannot call this functions from such + * interrupts. + * \return On a platform without processes this function returns false if no + * stack overflow was detected or causes a reboot if a stack overflow was + * detected. Basically, it never returns true. On a platform with processes + * return true if the stack overflow check failed for a thread running in + * userspace. If the overflow check failed for a kernel thread or a thread + * running in kernelspace this function causes a reboot. */ - static void IRQstackOverflowCheck(); + static bool IRQstackOverflowCheck(); #ifdef WITH_PROCESSES -- GitLab