[PATCH] sparc: Fix stack corruption
Martin Åberg
maberg at gaisler.com
Thu Oct 12 09:59:56 UTC 2023
Hello Sebastian,
I have reviewed the ticket and the patch. The fix is OK.
--
Best regards,
Martin Åberg
Software Engineer
Frontgrade Gaisler
martin.aberg at 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 );
>
More information about the devel
mailing list