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

[Hartmut Birr]

ion at svn.reactos.org 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 at 0
>  @KiSwapContextInternal at 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