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

[Hartmut Birr]

ion(a)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@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