diff --git a/miosix/arch/cpu/armv7m/interfaces-impl/userspace.cpp b/miosix/arch/cpu/armv7m/interfaces-impl/userspace.cpp
index 31bf533d871d8789f264f910ace779aad579520a..9598c423bcf0f897641304b2a0b45dcf41fcc19d 100644
--- a/miosix/arch/cpu/armv7m/interfaces-impl/userspace.cpp
+++ b/miosix/arch/cpu/armv7m/interfaces-impl/userspace.cpp
@@ -26,6 +26,7 @@
***************************************************************************/
#include "interfaces_private/userspace.h"
+#include "interfaces/cpu_const.h"
#include "kernel/error.h"
#include <cstdio>
#include <cstring>
@@ -83,8 +84,11 @@ void FaultData::print() const
case MP_XN:
iprintf("* Attempted instruction fetch @ 0x%x\n",pc);
break;
+ case MP_STACK:
+ iprintf("* Invalid SP (0x%x) while entering IRQ\n",arg);
+ break;
case UF_DIVZERO:
- iprintf("* Dvide by zero (PC was 0x%x)\n",pc);
+ iprintf("* Divide by zero (PC was 0x%x)\n",pc);
break;
case UF_UNALIGNED:
iprintf("* Unaligned memory access (PC was 0x%x)\n",pc);
@@ -116,6 +120,24 @@ void FaultData::print() const
}
}
+void FaultData::IRQtryAddProgramCounter(unsigned int *userCtxsave,
+ const MPUConfiguration& mpu)
+{
+ // ARM Cortex CPUs push the program counter on the stack. Thus we retrieve
+ // the stack pointer from userCtxsave and check it is not corrupted.
+ // We use mpu.withinForWriting even though the we're only reading from the
+ // stack frame since the stack pointer can't point to read-only memory, if
+ // it does it's corrupted. If the pointer checks out, we get the desired pc
+ const int pcOffsetInStackFrame=6;
+ auto *sp=reinterpret_cast<unsigned int*>(userCtxsave[STACK_OFFSET_IN_CTXSAVE]);
+ // NOTE: we check that (pcOffsetInStackFrame+1)*4 words are within the
+ // process address space instead of CTXSAVE_ON_STACK since in CPUs with
+ // FPU until a process uses FPU registers they are not saved, so it is
+ // possible that sp+CTXSAVE_ON_STACK coud overflow the process address space
+ if(mpu.withinForWriting(sp,(pcOffsetInStackFrame+1)*sizeof(unsigned int*)))
+ pc=sp[pcOffsetInStackFrame];
+}
+
//
// class MPUConfiguration
//
@@ -230,6 +252,8 @@ bool MPUConfiguration::withinForReading(const void *ptr, size_t size) const
bool MPUConfiguration::withinForWriting(const void *ptr, size_t size) const
{
+ //Must be callable also with interrupts disabled,
+ //used by FaultData::IRQtryAddProgramCounter()
size_t dataStart=regValues[2] & (~0x1f);
size_t dataEnd=dataStart+(1<<(((regValues[3]>>1) & 31)+1));
size_t base=reinterpret_cast<size_t>(ptr);
diff --git a/miosix/arch/cpu/armv7m/interfaces-impl/userspace_impl.h b/miosix/arch/cpu/armv7m/interfaces-impl/userspace_impl.h
index 6010db411b26f37c8d6968eea88273186c1181e5..3f79c961fa6fb88dbe12e02d512d564285e3d698 100644
--- a/miosix/arch/cpu/armv7m/interfaces-impl/userspace_impl.h
+++ b/miosix/arch/cpu/armv7m/interfaces-impl/userspace_impl.h
@@ -29,6 +29,7 @@
#include "config/miosix_settings.h"
#include "interfaces/arch_registers.h"
+#include "interfaces/cpu_const.h"
#include <cassert>
namespace miosix {
@@ -60,7 +61,7 @@ constexpr unsigned int armSyscallMapping[]={0,1,2,4};
//
inline SyscallParameters::SyscallParameters(unsigned int *context) :
- registers(reinterpret_cast<unsigned int*>(context[0])) {}
+ registers(reinterpret_cast<unsigned int*>(context[STACK_OFFSET_IN_CTXSAVE])) {}
inline int SyscallParameters::getSyscallId() const
{
@@ -95,16 +96,17 @@ enum FaultType
MP=2, //Process attempted data access outside its memory
MP_NOADDR=3, //Process attempted data access outside its memory (missing addr)
MP_XN=4, //Process attempted code access outside its memory
- UF_DIVZERO=5, //Process attempted to divide by zero
- UF_UNALIGNED=6, //Process attempted unaligned memory access
- UF_COPROC=7, //Process attempted a coprocessor access
- UF_EXCRET=8, //Process attempted an exception return
- UF_EPSR=9, //Process attempted to access the EPSR
- UF_UNDEF=10, //Process attempted to execute an invalid instruction
- UF_UNEXP=11, //Unexpected usage fault
- HARDFAULT=12, //Hardfault (for example process executed a BKPT instruction)
- BF=13, //Busfault
- BF_NOADDR=14, //Busfault (missing addr)
+ MP_STACK=5, //Process had invalid SP while entering IRQ
+ UF_DIVZERO=6, //Process attempted to divide by zero
+ UF_UNALIGNED=7, //Process attempted unaligned memory access
+ UF_COPROC=8, //Process attempted a coprocessor access
+ UF_EXCRET=9, //Process attempted an exception return
+ UF_EPSR=10, //Process attempted to access the EPSR
+ UF_UNDEF=11, //Process attempted to execute an invalid instruction
+ UF_UNEXP=12, //Unexpected usage fault
+ HARDFAULT=13, //Hardfault (for example process executed a BKPT instruction)
+ BF=14, //Busfault
+ BF_NOADDR=15 //Busfault (missing addr)
};
} //namespace fault
diff --git a/miosix/arch/cpu/common/cortexMx_interrupts.cpp b/miosix/arch/cpu/common/cortexMx_interrupts.cpp
index a2514b9e26c7cf42aee548699599732cf7cb4273..10e311bfa3c452d4ce1cf4394688f423a24e3f3e 100644
--- a/miosix/arch/cpu/common/cortexMx_interrupts.cpp
+++ b/miosix/arch/cpu/common/cortexMx_interrupts.cpp
@@ -299,24 +299,30 @@ static void printUnsignedInt(unsigned int x)
formatHex(result+2,x,8);
IRQerrorLog(result);
}
-#endif //WITH_ERRLOG
-#if defined(WITH_PROCESSES) || defined(WITH_ERRLOG)
/**
* \internal
- * \return the program counter of the thread that was running when the exception
- * occurred.
+ * \return attempt to get the program counter of the thread that was running
+ * when the exception occurred.
*/
-static unsigned int getProgramCounter()
+static unsigned int tryGetKernelThreadProgramCounter()
{
- register unsigned int result;
- // Get program counter when the exception was thrown from stack frame
- asm volatile("mrs %0, psp \n\t"
- "add %0, %0, #24 \n\t"
- "ldr %0, [%0] \n\t":"=r"(result));
- return result;
+ // Attempt to get program counter at the time the exception was thrown from
+ // stack frame. This can fail if the stack pointer itself is corrupted, in
+ // this case return 0xbadadd. Failing to validate the thread stack pointer
+ // before dereferencing it may cause further memory faults and a CPU lockup
+ unsigned int psp=__get_PSP();
+ // Miosix uses the stack-in-heap approach for kernel threads, so compare the
+ // stack pointer against the boundaries of the heap
+ extern char _end asm("_end"); //defined in the linker script
+ extern char _heap_end asm("_heap_end"); //defined in the linker script
+ const unsigned int off=24; //Offset in bytes of PC in the stack frame on ARM
+ if(psp<reinterpret_cast<unsigned int>(&_end)
+ || psp>reinterpret_cast<unsigned int>(&_heap_end)-off-sizeof(unsigned int))
+ return 0xbadadd;
+ return *reinterpret_cast<unsigned int *>(psp+off);
}
-#endif //WITH_PROCESSES || WITH_ERRLOG
+#endif //WITH_ERRLOG
//
// Interrupt handlers
@@ -402,15 +408,14 @@ void __attribute__((naked)) HardFault_Handler()
void __attribute__((noinline)) hardfaultImpl()
{
- if(Thread::IRQreportFault(FaultData(fault::HARDFAULT,getProgramCounter())))
- return;
+ if(Thread::IRQreportFault(FaultData(fault::HARDFAULT))) return;
#else //WITH_PROCESSES
void HardFault_Handler()
{
#endif //WITH_PROCESSES
#ifdef WITH_ERRLOG
IRQerrorLog("\r\n***Unexpected HardFault @ ");
- printUnsignedInt(getProgramCounter());
+ printUnsignedInt(tryGetKernelThreadProgramCounter());
#if __CORTEX_M != 0
unsigned int hfsr=SCB->HFSR;
if(hfsr & 0x40000000) //SCB_HFSR_FORCED
@@ -445,17 +450,29 @@ void MemManage_Handler()
int id, arg=0;
if(cfsr & 0x00000001) id=fault::MP_XN;
else if(cfsr & 0x00000080) { id=fault::MP; arg=SCB->MMFAR; }
+ else if(cfsr & 0x00000010) { id=fault::MP_STACK; arg=ctxsave[STACK_OFFSET_IN_CTXSAVE]; }
else id=fault::MP_NOADDR;
- if(Thread::IRQreportFault(FaultData(id,getProgramCounter(),arg)))
+ if(Thread::IRQreportFault(FaultData(id,arg)))
{
//Clear MMARVALID, MLSPERR, MSTKERR, MUNSTKERR, DACCVIOL, IACCVIOL
SCB->CFSR = 0x000000bb;
+ //Clear MEMFAULTPENDED bit. Corrupted thread stack pointer causes memory
+ //faults during exception stacking (MSTKERR bit), and if the core has an
+ //FPU and the thread was using the FPU registers attempting to save
+ //these registers causes a second memory fault (MSLPERR bit) which at
+ //least on an STM32H755 causes the memory fault interrupt to become both
+ //pending and active, thus it gets run twice. This is a problem since
+ //the first run reports the fault and switches the thread context from
+ //userspace to kernelspace, and then the second run finds the thread
+ //already in kernelspace and erroneously attributes the fault to code
+ //running in kernelspace triggering a reboot
+ SCB->SHCSR &= ~(1<<13);
return;
}
#endif //WITH_PROCESSES
#ifdef WITH_ERRLOG
IRQerrorLog("\r\n***Unexpected MemManage @ ");
- printUnsignedInt(getProgramCounter());
+ printUnsignedInt(tryGetKernelThreadProgramCounter());
if(cfsr & 0x00000080) //SCB_CFSR_MMARVALID
{
IRQerrorLog("Fault caused by attempted access to ");
@@ -493,16 +510,19 @@ void BusFault_Handler()
int id, arg=0;
if(cfsr & 0x00008000) { id=fault::BF; arg=SCB->BFAR; }
else id=fault::BF_NOADDR;
- if(Thread::IRQreportFault(FaultData(id,getProgramCounter(),arg)))
+ if(Thread::IRQreportFault(FaultData(id,arg)))
{
//Clear BFARVALID, LSPERR, STKERR, UNSTKERR, IMPRECISERR, PRECISERR, IBUSERR
SCB->CFSR = 0x0000bf00;
+ //Clear BUSFAULTPENDED as a defensive measure against issues similar to
+ //the one described in MEMFAULTPENDED
+ SCB->SHCSR &= ~(1<<14);
return;
}
#endif //WITH_PROCESSES
#ifdef WITH_ERRLOG
IRQerrorLog("\r\n***Unexpected BusFault @ ");
- printUnsignedInt(getProgramCounter());
+ printUnsignedInt(tryGetKernelThreadProgramCounter());
if(cfsr & 0x00008000) //SCB_CFSR_BFARVALID
{
IRQerrorLog("Fault caused by attempted access to ");
@@ -547,16 +567,19 @@ void UsageFault_Handler()
else if(cfsr & 0x00020000) id=fault::UF_EPSR;
else if(cfsr & 0x00010000) id=fault::UF_UNDEF;
else id=fault::UF_UNEXP;
- if(Thread::IRQreportFault(FaultData(id,getProgramCounter())))
+ if(Thread::IRQreportFault(FaultData(id)))
{
//Clear DIVBYZERO, UNALIGNED, UNDEFINSTR, INVSTATE, INVPC, NOCP
SCB->CFSR = 0x030f0000;
+ //Clear BUSFAULTPENDED as a defensive measure against issues similar to
+ //the one described in USGFAULTPENDED
+ SCB->SHCSR &= ~(1<<12);
return;
}
#endif //WITH_PROCESSES
#ifdef WITH_ERRLOG
IRQerrorLog("\r\n***Unexpected UsageFault @ ");
- printUnsignedInt(getProgramCounter());
+ printUnsignedInt(tryGetKernelThreadProgramCounter());
if(cfsr & 0x02000000) //SCB_CFSR_DIVBYZERO
IRQerrorLog("Divide by zero\r\n");
if(cfsr & 0x01000000) //SCB_CFSR_UNALIGNED
@@ -577,7 +600,7 @@ void DebugMon_Handler()
{
#ifdef WITH_ERRLOG
IRQerrorLog("\r\n***Unexpected DebugMon @ ");
- printUnsignedInt(getProgramCounter());
+ printUnsignedInt(tryGetKernelThreadProgramCounter());
#endif //WITH_ERRLOG
IRQsystemReboot();
}
@@ -594,51 +617,15 @@ void __attribute__((naked)) SVC_Handler()
void __attribute__((noinline)) svcImpl()
{
- // This fix is intended to avoid kernel or process faulting due to
- // another process actions. Consider the case in which a process statically
- // allocates a big array such that there is no space left for saving
- // context data. If the process issues a system call, in the following
- // interrupt the context is saved, but since there is no memory available
- // for all the context data, a mem manage interrupt is set to 'pending'. Then,
- // a fake syscall is issued, based on the value read on the stack (which
- // the process hasn't set due to the memory fault and is likely to be 0);
- // this syscall is usually a yield (due to the value of 0 above),
- // which can cause the scheduling of the kernel thread. At this point,
- // the pending mem fault is issued from the kernel thread, causing the
- // kernel fault and reboot. This is caused by the mem fault interrupt
- // having less priority of the other interrupts.
- // This fix checks if there is a mem fault interrupt pending, and, if so,
- // it clears it and returns before calling the previously mentioned fake
- // syscall.
- if(SCB->SHCSR & (1<<13))
- {
- if(Thread::IRQreportFault(FaultData(fault::MP,0,0)))
- {
- SCB->SHCSR &= ~(1<<13); //Clear MEMFAULTPENDED bit
- return;
- }
- }
- // 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
- //at this time we do not know if the active context is user or kernel
- unsigned int threadSp=ctxsave[0];
- unsigned int *processStack=reinterpret_cast<unsigned int*>(threadSp);
- if(processStack[3]==static_cast<unsigned int>(Syscall::YIELD))
- Scheduler::IRQrunScheduler();
- else Thread::IRQhandleSvc(processStack[3]);
+ Thread::IRQstackOverflowCheck();
+ Thread::IRQhandleSvc();
}
#else //WITH_PROCESSES
void SVC_Handler()
{
#ifdef WITH_ERRLOG
IRQerrorLog("\r\n***Unexpected SVC @ ");
- printUnsignedInt(getProgramCounter());
+ printUnsignedInt(tryGetKernelThreadProgramCounter());
#endif //WITH_ERRLOG
IRQsystemReboot();
}
diff --git a/miosix/interfaces_private/userspace.h b/miosix/interfaces_private/userspace.h
index dce444c51f4321a467979a5235121312f2bfa6c6..4386b53445fb751ce3a857dc7da73787970d55bf 100644
--- a/miosix/interfaces_private/userspace.h
+++ b/miosix/interfaces_private/userspace.h
@@ -50,6 +50,9 @@ namespace miosix {
#ifdef WITH_PROCESSES
+//Forward decl
+class MPUConfiguration;
+
/**
* \internal
* Initializes the context of the userspace side of a userspace thread that is
@@ -143,17 +146,32 @@ public:
/**
* Constructor, initializes a FaultData object
* \param id id of the fault
- * \param pc program counter at the moment of the fault
* \param arg eventual additional argument, depending on the fault id
*/
- FaultData(int id, unsigned int pc, unsigned int arg=0)
- : id(id), pc(pc), arg(arg) {}
+ explicit FaultData(int id, unsigned int arg=0) : id(id), arg(arg) {}
+
+ /**
+ * Try to reconstruct the value of the program counter at the moment of the
+ * fault and add it to the fault information. Depending on the architecture
+ * and how badly the process crashed, this may not be possible.
+ * \param userCtxsave saved context of the userspace thread that faulted
+ * \param mpu memory protection data for the current process to perform
+ * bound checking if required by the architecture
+ */
+ void IRQtryAddProgramCounter(unsigned int *userCtxsave,
+ const MPUConfiguration& mpu);
/**
* \return true if a fault happened within a process
*/
bool faultHappened() const { return id!=0; }
+ /**
+ * Can be called inside an interrupt
+ * \return true if a fault happened within a process
+ */
+ bool IRQfaultHappened() const { return id!=0; }
+
/**
* Print information about the occurred fault
*/
@@ -161,7 +179,7 @@ public:
private:
int id; ///< Id of the fault or zero if no faults
- unsigned int pc; ///< Program counter value at the time of the fault
+ unsigned int pc=0xbadadd; ///< Program counter value at the time of the fault
unsigned int arg;///< Eventual argument, valid only for some id values
};
diff --git a/miosix/kernel/kernel.cpp b/miosix/kernel/kernel.cpp
index 4fbd36a7d1eb8fb8b56210be9d66580cad9b5fd0..b7e36a64f8420955e99d95a3d2b1d38de83aa959 100755
--- a/miosix/kernel/kernel.cpp
+++ b/miosix/kernel/kernel.cpp
@@ -617,57 +617,72 @@ int Thread::getStackSize()
return getCurrentThread()->stacksize;
}
-bool Thread::IRQstackOverflowCheck()
+void Thread::IRQstackOverflowCheck()
{
+ Thread *cur=const_cast<Thread*>(runningThread);
const unsigned int watermarkSize=WATERMARK_LEN/sizeof(unsigned int);
#ifdef WITH_PROCESSES
- if(const_cast<Thread*>(runningThread)->flags.isInUserspace())
+ if(cur->flags.isInUserspace())
{
bool overflow=false;
- if(runningThread->userCtxsave[STACK_OFFSET_IN_CTXSAVE] <
- reinterpret_cast<unsigned int>(runningThread->userWatermark+watermarkSize))
+ if(cur->userCtxsave[STACK_OFFSET_IN_CTXSAVE] <
+ reinterpret_cast<unsigned int>(cur->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));
- return overflow;
+ if(cur->userWatermark[i]!=WATERMARK_FILL) overflow=true;
+ if(overflow) IRQreportFault(FaultData(fault::STACKOVERFLOW));
}
#endif //WITH_PROCESSES
- if(runningThread->ctxsave[STACK_OFFSET_IN_CTXSAVE] <
- reinterpret_cast<unsigned int>(runningThread->watermark+watermarkSize))
+ if(cur->ctxsave[STACK_OFFSET_IN_CTXSAVE] <
+ reinterpret_cast<unsigned int>(cur->watermark+watermarkSize))
errorHandler(STACK_OVERFLOW);
for(unsigned int i=0;i<watermarkSize;i++)
- if(runningThread->watermark[i]!=WATERMARK_FILL) errorHandler(STACK_OVERFLOW);
- return false;
+ if(cur->watermark[i]!=WATERMARK_FILL) errorHandler(STACK_OVERFLOW);
}
#ifdef WITH_PROCESSES
-void Thread::IRQhandleSvc(unsigned int svcNumber)
+void Thread::IRQhandleSvc()
{
- if(runningThread->proc==kernel) errorHandler(UNEXPECTED);
+ Thread *cur=const_cast<Thread*>(runningThread);
+ if(cur->proc==kernel) errorHandler(UNEXPECTED);
+ //We know it's not the kernel, so the cast is safe
+ auto *proc=static_cast<Process*>(cur->proc);
+ //Don't process syscall if fault happened, may return to userspace by mistake
+ if(proc->fault.IRQfaultHappened()) return;
+
+ //Note that it is required to use ctxsave and not runningThread->ctxsave
+ //because at this time we do not know if the active context is user or kernel
+ SyscallParameters params(const_cast<unsigned int*>(::ctxsave));
+ unsigned int svcNumber=params.getSyscallId();
+ if(svcNumber==static_cast<unsigned int>(Syscall::YIELD))
+ {
+ Scheduler::IRQrunScheduler();
+ return;
+ }
if(svcNumber==static_cast<unsigned int>(Syscall::USERSPACE))
{
- const_cast<Thread*>(runningThread)->flags.IRQsetUserspace(true);
- ::ctxsave=runningThread->userCtxsave;
- //We know it's not the kernel, so the cast is safe
- static_cast<Process*>(runningThread->proc)->mpu.IRQenable();
+ cur->flags.IRQsetUserspace(true);
+ ::ctxsave=cur->userCtxsave;
+ proc->mpu.IRQenable();
} else {
- const_cast<Thread*>(runningThread)->flags.IRQsetUserspace(false);
- ::ctxsave=runningThread->ctxsave;
+ cur->flags.IRQsetUserspace(false);
+ ::ctxsave=cur->ctxsave;
MPUConfiguration::IRQdisable();
}
}
bool Thread::IRQreportFault(const FaultData& fault)
{
- if(const_cast<Thread*>(runningThread)->flags.isInUserspace()==false
- || runningThread->proc==kernel) return false;
+ Thread *cur=const_cast<Thread*>(runningThread);
+ if(cur->flags.isInUserspace()==false || cur->proc==kernel) return false;
//We know it's not the kernel, so the cast is safe
- static_cast<Process*>(runningThread->proc)->fault=fault;
- const_cast<Thread*>(runningThread)->flags.IRQsetUserspace(false);
- ::ctxsave=runningThread->ctxsave;
+ auto *proc=static_cast<Process*>(cur->proc);
+ proc->fault=fault;
+ proc->fault.IRQtryAddProgramCounter(cur->userCtxsave,proc->mpu);
+ cur->flags.IRQsetUserspace(false);
+ ::ctxsave=cur->ctxsave;
MPUConfiguration::IRQdisable();
return true;
}
diff --git a/miosix/kernel/kernel.h b/miosix/kernel/kernel.h
index 5033e5d5f04334155b132c337a2efdd00917bf4e..36b76fdfdae15608cce72e897bd9083fb8df9247 100755
--- a/miosix/kernel/kernel.h
+++ b/miosix/kernel/kernel.h
@@ -893,14 +893,13 @@ public:
* 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.
+ *
+ * If the overflow check failed for a kernel thread or a thread running in
+ * kernelspace this function causes a reboot. On a platform with processes
+ * this function calls IRQreportFault() if the stack overflow happened in
+ * userspace, causing the process to segfault.
*/
- static bool IRQstackOverflowCheck();
+ static void IRQstackOverflowCheck();
#ifdef WITH_PROCESSES
@@ -914,7 +913,7 @@ public:
* Can only be called inside an IRQ, its use is to switch a thread between
* userspace/kernelspace and back to perform context switches
*/
- static void IRQhandleSvc(unsigned int svcNumber);
+ static void IRQhandleSvc();
/**
* \internal