Re: [ros-diffs] [ion] 20911: - Fix some nasty context switch bugs:

[Hartmut Birr]

[e-mail] wrote:
> Fix some nasty context switch bugs:
> We did not update the KPCR's stacklimit/initialstack with the new thread's stacklimit/initialstack.
> We always assumed V86 frame bias in KeInitializeThreadContext.
> We did not properly update ESP0 during context switch, to make space for the NPX frame and V86 bias.
> We did not update fs:18h to point to the new TEB.
> We did not clear out GS when switching processes, nor update the TSS's cr3.
> If a new LDT was being updated, we over-wrote EBP (which was supposed to point to the TSS) by the GDT pointer.
> We used a push/pop esp0 hack which hid the fact we never updated esp0.
>
> Modified: trunk/reactos/ntoskrnl/ke/i386/ctxswitch.S
> Modified: trunk/reactos/ntoskrnl/ke/i386/thread.c
>
> *Modified: trunk/reactos/ntoskrnl/ke/i386/ctxswitch.S*
> @@ -114,10 +114,6 @@
>
> *--*/
>  .globl @KiSwapContextInternal@0
>  @KiSwapContextInternal@0:
>
> -#ifdef KDBG
> //jmp SaveTrapFrameForKDB
>
> -SaveTrapFrameForKDB_Return:
> -#endif
>  /* Get the PCR. It's faster to use ebx+offset then fs:offset */
>  mov ebx, [fs:KPCR_SELF]
>
> @@ -132,24 +128,36 @@
>  cli
>
>  /* Save the initial stack in EAX */
>
> mov eax, [edi+KTHREAD_INITIAL_STACK]
> mov eax, [esi+KTHREAD_INITIAL_STACK]

Now eax points to the trap frame of the thread we will switch to.
> /* Save the stack limit in ecx */
> mov ecx, [esi+KTHREAD_STACK_LIMIT]
> /* Make space for the NPX Frame */
> sub eax, NPX_FRAME_LENGTH
> /* Set the KPCR stack values */
> mov [ebx+KPCR_INITIAL_STACK], eax
> mov [ebx+KPCR_STACK_LIMIT], ecx
>
> #ifdef CONFIG_SMP
>      /* Save FPU state if the thread has used it. */
>
> mov ecx, [edi+KTHREAD_INITIAL_STACK]
>
> sub ecx, NPX_FRAME_LENGTH
>
> mov dword ptr [ebx+KPCR_NPX_THREAD], 0
> test byte ptr [edi+KTHREAD_NPX_STATE], NPX_STATE_DIRTY
> jz 3f
> cmp dword ptr _KeI386FxsrPresent, 0
> je 1f
> fxsave [eax-SIZEOF_FX_SAVE_AREA]
>
> fxsave [ecx]
>
> jmp 2f
>
> 1:
> fnsave [eax-SIZEOF_FX_SAVE_AREA]
> fnsave [ecx]
>
> 2:
>      mov byte ptr [edi+KTHREAD_NPX_STATE], NPX_STATE_VALID
>  3:
>  #endif /* CONFIG_SMP */
>
> /* Save the stack pointer in this processors TSS */
>    mov ebp, [ebx+KPCR_TSS]
>
> @@ -158,12 +166,17 @@
>  jnz NoAdjust

The compare operation uses eax which points to the thread we will switch
to. We must use the trap frame from the thread we will switch from.
>  /* Bias esp */
>
> //sub dword ptr ss:[ebp+KTSS_ESP0], KTRAP_FRAME_V86_GS - KTRAP_FRAME_SS
> sub eax, KTRAP_FRAME_V86_GS - KTRAP_FRAME_SS

Eax points to the wrong stack. See above.
> NoAdjust:
> /* Push ESP0 Value */
> push ss:[ebp+KTSS_ESP0]
> /* Set new ESP0 */
> mov [ebp+KTSS_ESP0], eax

Now, we are using the wrong kernel stack on the next exception.

- Hartmut