Hello Sebastian,
I have reviewed the ticket and the patch. The fix is OK.
--
Best regards,
Martin Åberg
Software Engineer
Frontgrade Gaisler
martin.ab...@gaisler.com
Frontgrade Gaisler AB, Kungsgatan 12, SE-411 19 GÖTEBORG, Sweden.
+46 (0) 31 775 8650, www.gaisler.com
On 2023-09-20 09:38, Sebastian Huber wrote:
Fix a potential stack corruption in uniprocessor configurations during
start multitasking .
The system initialization uses the interrupt stack. A first level
interrupt shall never interrupt a context which uses the interrupt
stack. Such a use would lead to stack corruption and undefined system
behaviour. Unfortunately, in uniprocessor configurations this was the
case. Multiprocessing is started using _CPU_Context_restore(). The
caller of this function (_Thread_Start_multitasking()) uses the
interrupt stack. Later we have in cpukit/score/cpu/sparc/cpu_asm.S:
mov %g1, %psr ! restore status register and
! **** ENABLE TRAPS ****
ld [%o1 + G5_OFFSET], %g5 ! restore the global registers
ld [%o1 + G7_OFFSET], %g7
! Load thread specific ISR dispatch prevention flag
ld [%o1 + ISR_DISPATCH_DISABLE_STACK_OFFSET], %o2
! Store it to memory later to use the cycles
ldd [%o1 + L0_OFFSET], %l0 ! restore the local registers
ldd [%o1 + L2_OFFSET], %l2
ldd [%o1 + L4_OFFSET], %l4
ldd [%o1 + L6_OFFSET], %l6
! Now restore thread specific ISR dispatch prevention flag
st %o2, [%g6 + PER_CPU_ISR_DISPATCH_DISABLE]
ldd [%o1 + I0_OFFSET], %i0 ! restore the input registers
ldd [%o1 + I2_OFFSET], %i2
ldd [%o1 + I4_OFFSET], %i4
ldd [%o1 + I6_FP_OFFSET], %i6
ldd [%o1 + O6_SP_OFFSET], %o6 ! restore the output registers
Between the ENABLE TRAPS and the restore of the output registers, we
still use the stack of the caller and interrupts may be enabled. If an
interrupt happens in this code block, the interrupt stack is
concurrently used which may lead to a crash.
Fix this by adding a new function _SPARC_Start_multiprocessing() for
uniprocessor configurations. This function first sets the stack pointer
to use the stack of the heir thread.
Close #4955.
---
cpukit/score/cpu/sparc/cpu_asm.S | 29 ++++++++++++++++++-
.../score/cpu/sparc/include/rtems/score/cpu.h | 19 ++++++++++++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/cpukit/score/cpu/sparc/cpu_asm.S b/cpukit/score/cpu/sparc/cpu_asm.S
index 287c2c4cd9..fd7186b499 100644
--- a/cpukit/score/cpu/sparc/cpu_asm.S
+++ b/cpukit/score/cpu/sparc/cpu_asm.S
@@ -246,6 +246,14 @@ done_flushing:
mov %g1, %psr ! restore status register and
! **** ENABLE TRAPS ****
+ /*
+ * WARNING: This code does not run with the restored stack pointer. In
+ * SMP configurations, it uses a processor-specific stack. In
+ * uniprocessor configurations, it uses the stack of the caller. In
+ * this case, the caller shall ensure that it is not the interrupt
+ * stack (which is also the system initialization stack).
+ */
+
ld [%o1 + G5_OFFSET], %g5 ! restore the global registers
ld [%o1 + G7_OFFSET], %g7
@@ -266,7 +274,9 @@ done_flushing:
ldd [%o1 + I4_OFFSET], %i4
ldd [%o1 + I6_FP_OFFSET], %i6
- ldd [%o1 + O6_SP_OFFSET], %o6 ! restore the output registers
+ ldd [%o1 + O6_SP_OFFSET], %o6 ! restore the non-volatile output
+ ! registers (stack pointer,
+ ! link register)
jmp %o7 + 8 ! return
nop ! delay slot
@@ -325,6 +335,23 @@ SYM(_CPU_Context_restore):
ba SYM(_CPU_Context_restore_heir)
mov %i0, %o1 ! in the delay slot
+#if !defined(RTEMS_SMP)
+ .align 4
+ PUBLIC(_SPARC_Start_multitasking)
+SYM(_SPARC_Start_multitasking):
+ /*
+ * Restore the stack pointer right now, so that the window flushing and
+ * interrupts during _CPU_Context_restore_heir() use the stack of the
+ * heir thread. This is crucial for the interrupt handling to prevent
+ * a concurrent use of the interrupt stack (which is also the system
+ * initialization stack).
+ */
+ ld [%o0 + O6_SP_OFFSET], %o6
+
+ ba SYM(_CPU_Context_restore)
+ nop
+#endif
+
/*
* void _SPARC_Interrupt_trap()
*
diff --git a/cpukit/score/cpu/sparc/include/rtems/score/cpu.h
b/cpukit/score/cpu/sparc/include/rtems/score/cpu.h
index 2021b108db..a21cef371f 100644
--- a/cpukit/score/cpu/sparc/include/rtems/score/cpu.h
+++ b/cpukit/score/cpu/sparc/include/rtems/score/cpu.h
@@ -993,6 +993,25 @@ RTEMS_NO_RETURN void _CPU_Context_switch_no_return(
*/
RTEMS_NO_RETURN void _CPU_Context_restore( Context_Control *new_context );
+#if !defined(RTEMS_SMP)
+/**
+ * @brief Starts multitasking in uniprocessor configurations.
+ *
+ * This function just sets the stack of the heir thread and then calls
+ * _CPU_Context_restore().
+ *
+ * This is causes that the window flushing and interrupts during
+ * _CPU_Context_restore() use the stack of the heir thread. This is crucial
+ * for the interrupt handling to prevent a concurrent use of the interrupt
+ * stack (which is also the system initialization stack).
+ *
+ * @param[in] heir is the context of the heir thread.
+ */
+RTEMS_NO_RETURN void _SPARC_Start_multitasking( Context_Control *heir );
+
+#define _CPU_Start_multitasking( _heir ) _SPARC_Start_multitasking( _heir )
+#endif
+
#if defined(RTEMS_SMP)
uint32_t _CPU_SMP_Initialize( void );
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
