Hi,
XenOS wrote:Brendan wrote:The other thing that worries me is race conditions caused by an NMI that occurs at exactly the wrong time (after CPL=3 code was interrupted by any other interrupt handler but before the first interrupt handler has finished saving the interrupted thread's state and switching to the kernel's stack). Without adequate work-arounds, the consequences of the race conditions may range from "severe" to "worse than severe", depending on the specific code being used for normal interrupt handlers and the NMI handler.
How could one solve this kind of problem? If I see this correctly, the problem is that the NMI handler would save some registers on the stack, which is in fact a small portion of the TCB, and it would thus trash some parts of the TCB which follow the register save area. So the problem is mainly caused by putting the stack into the TCB, where it becomes fatal if it grows more than expected. I guess the NMI handler needs to switch to its own stack immediately or at least detect that the current stack pointer is not safe for extensive pushes...
I've been thinking about it. I've decided/realised that the problems don't just effect NMI - they effect any exception that could occur at exactly the wrong time, which could include the machine check exception and possibly others.
For one example, a kernel might assume GS always points to the CPU's "per CPU data area" and then arrange things such that if (malicious) CPL=3 code changed GS to something else a general protection fault or page fault will occur when the kernel attempts to use GS, and the exception handler corrects GS and returns (such that the GS is auto-corrected if/when necessary).
Note: While this sounds complex it is a viable optimisation in some cases (it avoids the need for the kernel to ever load GS when there's no malicious code running, and simplifies things like "mov esi,[gs:CPUdata.currentTCB]" in interrupt handlers).
In addition; I've realised that there may also be problems returning from an interrupt; where an NMI (or other exception) could occur while you're in the middle of returning to CPL=3.
The exact number of problems and their effects depends on the specific code being used for any interrupt handlers involved; and therefore it's a little hard to find solutions without having specific code to analyse. With this in mind, here's some (untested) example code I made up. To make it more fun I'm relying on the GS trick mentioned above and showing code for protected mode (where you can't just use "swapgs"). Also note that I don't bother saving/restoring CPL=3 DS and ES because (for my OS) user space code shouldn't modify them (and for all I care user space code can/should crash if it does modify them).
Code: Select all
interruptHandler:
push dword 0 ;Dummy error code (remove for some exception handlers)
test byte [esp+4],3 ;Was interrupted code running at CPL=0?
je .isCPL0 ; yes
;Save remaining CPL=3 state
pushad
;Make sure kernel data segments are being used
mov eax,KERNEL_DATA
mov ds,eax
mov es,eax
;Switch to the kernel's stack
mov esp,[gs:CPUinfo.kernelStack]
;Enable interrupts (assume interrupt handler uses an "interrupt gate")
sti
;Call the real interrupt handler, C calling conventions for "void realInterruptHandler(uint32_t interruptNumber)"
push 123 ;Assume this interrupt handler is for interrupt number 123 (!)
call realInterruptHandler
add esp,4
;Disable interrupts
cli
;Restore CPL3 state (may be returning to a different thread)
mov edx,cr3
mov eax,[gs:CPUinfo.currentTCB] ;eax = address of TCB for this thread
cmp edx,[eax+TCB.userCR3] ;Is virtual address space switch needed?
je .l1 ; no, skip it
mov edx,[eax+TCB.userCR3]
mov cr3,edx
.l1:
mov edi,[gs:CPUinfo.TSSaddress] ;edi = address of TSS for this CPU
lea ebx,[eax+offsetForESP0] ;ebx = value to put in TSS for this thread
mov [edi+TSS.ESP0],eax ;Set ESP0 in TSS
lea esp,[eax+offsetForReturnData] ;Switch to TCB stack to POP return values
popad
add esp,4 ;Remove error code
iretd
.isCPL0:
push dword [esp+16] ;Push "return EFLAGS" on the stack again
popfd ;Restore the interrupted code's "interrupt enable" flag
pushad
push 123 ;Assume this interrupt handler is for interrupt number 123 (!)
call realInterruptHandler
add esp,4
popad
cli
iretd
There are 4 problems here:
- If an NMI (or some other exception) occurs at the wrong time at the start of an interrupt handler, that exception (and *all* exceptions that interrupt that exception) can trash data in the TCB and maybe beyond the end of the TCB too.
- If an NMI (or some other exception) occurs at the wrong time at the start of an interrupt handler, then the exception handler would think it interrupted CPL=0 code but can't assume that DS or ES are sane
- If an NMI (or some other exception) occurs at the wrong time at the start of an interrupt handler, then the exception handler would think it interrupted CPL=0 code but can't assume its stack is sane
- If an NMI (or some other exception) occurs at the wrong time at the end of an interrupt handler (e.g. when a normal interrupt handler is in the middle of returning to CPL=3), then the exception handler would think it interrupted CPL=0 code but can't assume it's stack is sane
The solution to problem 2) is that the exception handler for any exception that could occur at the wrong time needs to load DS and ES regardless of whether the exception occured when kernel code is running or not.
The solution to problems 1), 3) and 4) is simple: don't use the TCB for a stack to begin with.
The code above would be rewritten as:
Code: Select all
interruptHandler:
push dword 0 ;Dummy error code (remove for some exception handlers)
test byte [esp+8],3 ;Was interrupted code running at CPL=0?
je .isCPL0 ; yes
mov [gs:CPUinfo.tempUserEAX],eax ;Store EAX somewhere safe
mov eax,KERNEL_DATA ;Make sure DS and ES are correct
mov ds,eax
mov es,eax
mov [gs:CPUinfo.tempUserEBX],ebx ;Store EBX somewhere safe
mov eax,[gs:CPUinfo.currentTCB] ;eax = address of interrupted thread's TCB
mov ebx,[esp+4] ;ebx = "return EIP"
mov [eax+TCB.userEIP],ebx
mov ebx,[esp+12] ;ebx = "return ESP"
mov [eax+TCB.userESP],ebx
mov ebx,[gs:CPUinfo.tempUserEAX] ;ebx = user EAX
mov [eax+TCB.userEAX],ebx
mov ebx,[gs:CPUinfo.tempUserEBX] ;ebx = user EBX
mov [eax+TCB.userEBX],ebx
mov [eax+TCB.userECX],ecx
mov [eax+TCB.userEDX],edx
mov [eax+TCB.userEBP],ebp
mov [eax+TCB.userESI],esi
mov [eax+TCB.userEDI],edi
;Enable interrupts (assume interrupt handler uses an "interrupt gate")
sti
;Call the real interrupt handler, C calling conventions for "void realInterruptHandler(uint32_t interruptNumber)"
push 123 ;Assume this interrupt handler is for interrupt number 123 (!)
call realInterruptHandler
add esp,4
;Disable interrupts
cli
;Restore CPL3 state (may be returning to a different thread)
mov edx,cr3
mov eax,[gs:CPUinfo.currentTCB] ;eax = address of TCB for this thread
cmp edx,[eax+TCB.userCR3] ;Is virtual address space switch needed?
je .l1 ; no, skip it
mov edx,[eax+TCB.userCR3]
mov cr3,edx
.l1:
mov ebx,[eax+TBC.userEIP]
mov ecx,[eax+TBC.userESP]
mov [esp+4],ebx ;Set "return EIP"
mov [esp+12],ebx ;Set "return ESP"
mov ebx,[eax+TCB.userEBX]
mov ecx,[eax+TCB.userECX]
mov edx,[eax+TCB.userEDX]
mov ebp,[eax+TCB.userEBP]
mov esi,[eax+TCB.userESI]
mov edi,[eax+TCB.userEDI]
mov eax,[eax+TCB.userEAX]
add esp,4 ;Remove error code
iretd
.isCPL0:
push dword [esp+16] ;Push "return EFLAGS" on the stack again
popfd ;Restore the interrupted code's "interrupt enable" flag
pushad
mov eax,KERNEL_DATA
mov ds,eax
mov es,eax
push 123 ;Assume this interrupt handler is for interrupt number 123 (!)
call realInterruptHandler
add esp,4
popad
iretd
Cheers,
Brendan
