From 05456808e8a09346f9abdb7ca99919eb1d93cbc1 Mon Sep 17 00:00:00 2001 From: Timo Kreuzer Date: Thu, 15 Dec 2022 21:55:16 +0200 Subject: [PATCH] [NTOS:KE/x64] Fix handling of non-volatiles in trap vs exception frame The registers that are saved/restored in the trap / exception frame need to be consistent between all entry/exit points as well as the functions that convert between trap/exception frame and context. The trap frame contains only the non-volatile registers and rbp, the rest is saved in the exception frame. The previous code didn't save rbp in the syscall handler, which led to it being clobbered when exiting though KiServiceExit2 rather than returning back to the syscall exit path. Also KeContextToTrapFrame would use rbx, rsi and rdi from the trap frame, which wouldn't be saved there by the syscall handler. --- ntoskrnl/ke/amd64/context.c | 6 +++--- ntoskrnl/ke/amd64/trap.S | 4 +++- sdk/include/asm/trapamd64.inc | 17 ----------------- 3 files changed, 6 insertions(+), 21 deletions(-) diff --git a/ntoskrnl/ke/amd64/context.c b/ntoskrnl/ke/amd64/context.c index 1e9900af11f..3c345f89052 100644 --- a/ntoskrnl/ke/amd64/context.c +++ b/ntoskrnl/ke/amd64/context.c @@ -36,11 +36,8 @@ KeContextToTrapFrame(IN PCONTEXT Context, if (ContextFlags & CONTEXT_INTEGER) { TrapFrame->Rax = Context->Rax; - TrapFrame->Rbx = Context->Rbx; TrapFrame->Rcx = Context->Rcx; TrapFrame->Rdx = Context->Rdx; - TrapFrame->Rsi = Context->Rsi; - TrapFrame->Rdi = Context->Rdi; TrapFrame->Rbp = Context->Rbp; TrapFrame->R8 = Context->R8; TrapFrame->R9 = Context->R9; @@ -48,6 +45,9 @@ KeContextToTrapFrame(IN PCONTEXT Context, TrapFrame->R11 = Context->R11; if (ExceptionFrame) { + ExceptionFrame->Rbx = Context->Rbx; + ExceptionFrame->Rsi = Context->Rsi; + ExceptionFrame->Rdi = Context->Rdi; ExceptionFrame->R12 = Context->R12; ExceptionFrame->R13 = Context->R13; ExceptionFrame->R14 = Context->R14; diff --git a/ntoskrnl/ke/amd64/trap.S b/ntoskrnl/ke/amd64/trap.S index a2cbd4ab013..6013c5492d0 100644 --- a/ntoskrnl/ke/amd64/trap.S +++ b/ntoskrnl/ke/amd64/trap.S @@ -781,6 +781,7 @@ PUBLIC KiSystemCallEntry64 /* The unwind info pretends we have a machine frame */ .PUSHFRAME .ALLOCSTACK (KTRAP_FRAME_LENGTH + MAX_SYSCALL_PARAM_SIZE - MachineFrameLength) + .SAVEREG rbp, MAX_SYSCALL_PARAM_SIZE + KTRAP_FRAME_Rbp .ENDPROLOG /* Swap gs to kernel, so we can access the PCR */ @@ -795,7 +796,7 @@ PUBLIC KiSystemCallEntry64 /* Allocate a TRAP_FRAME and space for parameters */ sub rsp, (KTRAP_FRAME_LENGTH + MAX_SYSCALL_PARAM_SIZE) - /* Save volatile registers in the trap frame */ + /* Save volatile registers and rbp in the trap frame */ mov [rsp + MAX_SYSCALL_PARAM_SIZE + KTRAP_FRAME_Rax], rax mov [rsp + MAX_SYSCALL_PARAM_SIZE + KTRAP_FRAME_Rip], rcx mov [rsp + MAX_SYSCALL_PARAM_SIZE + KTRAP_FRAME_Rdx], rdx @@ -803,6 +804,7 @@ PUBLIC KiSystemCallEntry64 mov [rsp + MAX_SYSCALL_PARAM_SIZE + KTRAP_FRAME_R9], r9 mov [rsp + MAX_SYSCALL_PARAM_SIZE + KTRAP_FRAME_Rcx], r10 mov [rsp + MAX_SYSCALL_PARAM_SIZE + KTRAP_FRAME_EFlags], r11 + mov [rsp + MAX_SYSCALL_PARAM_SIZE + KTRAP_FRAME_Rbp], rbp /* Store user stack pointer in the trap frame */ mov rax, gs:[PcUserRsp] diff --git a/sdk/include/asm/trapamd64.inc b/sdk/include/asm/trapamd64.inc index 0c09063f058..b095f193481 100644 --- a/sdk/include/asm/trapamd64.inc +++ b/sdk/include/asm/trapamd64.inc @@ -108,16 +108,6 @@ MACRO(EnterTrap, Flags) lea rbp, [rsp] .setframe rbp, 0 - if (Flags AND TF_NONVOLATILES) - /* Save non-volatile registers */ - mov [rbp + KTRAP_FRAME_Rbx], rbx - .savereg rbx, KTRAP_FRAME_Rbx - mov [rbp + KTRAP_FRAME_Rdi], rdi - .savereg rdi, KTRAP_FRAME_Rdi - mov [rbp + KTRAP_FRAME_Rsi], rsi - .savereg rsi, KTRAP_FRAME_Rsi - endif - .endprolog if (Flags AND TF_VOLATILES) @@ -243,13 +233,6 @@ MACRO(ExitTrap, Flags) kernel_mode_return: - if (Flags AND TF_NONVOLATILES) - /* Restore non-volatile registers */ - mov rbx, [rbp + KTRAP_FRAME_Rbx] - mov rdi, [rbp + KTRAP_FRAME_Rdi] - mov rsi, [rbp + KTRAP_FRAME_Rsi] - endif - if (Flags AND TF_VOLATILES) /* Restore volatile registers */ mov rax, [rbp + KTRAP_FRAME_Rax]