[PATCH v2 0/4] MR9060: ntdll: Fix crashes in syscall dispatcher due to attempt to restore unitialized data.
-- v2: ntdll: Handle invalid FP state in usr1_handler() on i386. https://gitlab.winehq.org/wine/wine/-/merge_requests/9060
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/signal_x86_64.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 9e02d2cb9d0..8bc39f35f05 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -1723,6 +1723,10 @@ __ASM_GLOBAL_FUNC( call_user_mode_callback, "leaq 0x10(%rbp),%rax\n\t" "movq %rax,0xa8(%rsp)\n\t" /* frame->syscall_cfa */ "movq 0x378(%r13),%r14\n\t" /* thread_data->syscall_frame */ + "movq 0x78(%r14),%rax\n\t" /* prev_frame->cs */ + "movq %rax,0x78(%rsp)\n\t" /* frame->cs */ + "movq 0x90(%r14),%rax\n\t" /* prev_frame->ss */ + "movq %rax,0x90(%rsp)\n\t" /* frame->ss */ "movq %r14,0xa0(%rsp)\n\t" /* frame->prev_frame */ "movq %rsp,0x378(%r13)\n\t" /* thread_data->syscall_frame */ "testl $1,0x380(%r13)\n\t" /* thread_data->syscall_trace */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9060
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/signal_i386.c | 4 ++++ dlls/ntdll/unix/signal_x86_64.c | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 92a01bb37cb..d628d44272d 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2700,6 +2700,10 @@ __ASM_GLOBAL_FUNC( __wine_unix_call_dispatcher, "movl $0,(%ecx)\n\t" /* frame->restore_flags */ "popl 0x08(%ecx)\n\t" /* frame->eip */ __ASM_CFI(".cfi_adjust_cfa_offset -4\n\t") + "pushfl\n\t" + __ASM_CFI(".cfi_adjust_cfa_offset 4\n\t") + "popl 0x04(%ecx)\n\t" /* frame->eflags */ + __ASM_CFI(".cfi_adjust_cfa_offset -4\n\t") __ASM_CFI_REG_IS_AT1(eip, ecx, 0x08) __ASM_GLOBL(__ASM_NAME("__wine_unix_call_dispatcher_prolog_end")) "\n\t" "leal 0x10(%esp),%edx\n\t" diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 8bc39f35f05..6564074893a 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -3140,6 +3140,10 @@ __ASM_GLOBAL_FUNC( __wine_unix_call_dispatcher, "movq %gs:0x378,%rcx\n\t" /* thread_data->syscall_frame */ "popq 0x70(%rcx)\n\t" /* frame->rip */ __ASM_CFI(".cfi_adjust_cfa_offset -8\n\t") + "pushfq\n\t" + __ASM_CFI(".cfi_adjust_cfa_offset 8\n\t") + "popq 0x80(%rcx)\n\t" /* frame->eflags */ + __ASM_CFI(".cfi_adjust_cfa_offset -8\n\t") __ASM_CFI_REG_IS_AT2(rip, rcx, 0xf0,0x00) "movl $0,0xb4(%rcx)\n\t" /* frame->restore_flags */ __ASM_LOCAL_LABEL("__wine_unix_call_dispatcher_prolog_end") ":\n\t" -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9060
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/signal_x86_64.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 6564074893a..2cd1c8a3444 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -439,6 +439,7 @@ C_ASSERT( offsetof(struct callback_stack_layout, machine_frame) == 0x30 ); C_ASSERT( sizeof(struct callback_stack_layout) == 0x58 ); #define RESTORE_FLAGS_INSTRUMENTATION CONTEXT_i386 +#define RESTORE_FLAGS_INVALID_FPSTATE CONTEXT_ARM struct syscall_frame { @@ -2069,6 +2070,7 @@ static BOOL handle_syscall_trap( ucontext_t *sigcontext, siginfo_t *siginfo ) extern const void *__wine_syscall_dispatcher_prolog_end_ptr; RIP_sig( sigcontext ) = (ULONG64)__wine_syscall_dispatcher_prolog_end_ptr; + frame->restore_flags = CONTEXT_CONTROL; } else if ((void *)RIP_sig( sigcontext ) == __wine_unix_call_dispatcher) { @@ -2076,6 +2078,7 @@ static BOOL handle_syscall_trap( ucontext_t *sigcontext, siginfo_t *siginfo ) RIP_sig( sigcontext ) = (ULONG64)__wine_unix_call_dispatcher_prolog_end_ptr; R10_sig( sigcontext ) = RCX_sig( sigcontext ); + frame->restore_flags = CONTEXT_CONTROL | RESTORE_FLAGS_INVALID_FPSTATE; } else if (siginfo->si_code == 4 /* TRAP_HWBKPT */ && is_inside_syscall( RSP_sig(sigcontext) )) { @@ -2089,7 +2092,6 @@ static BOOL handle_syscall_trap( ucontext_t *sigcontext, siginfo_t *siginfo ) frame->rip = *(ULONG64 *)RSP_sig( sigcontext ); frame->eflags = EFL_sig(sigcontext); - frame->restore_flags = CONTEXT_CONTROL; if (instrumentation_callback) frame->restore_flags |= RESTORE_FLAGS_INSTRUMENTATION; RCX_sig( sigcontext ) = (ULONG64)frame; @@ -2430,6 +2432,24 @@ static void usr1_handler( int signal, siginfo_t *siginfo, void *sigcontext ) return; } context->c.ContextFlags = CONTEXT_FULL | CONTEXT_SEGMENTS | CONTEXT_EXCEPTION_REQUEST; + if (frame->restore_flags & RESTORE_FLAGS_INVALID_FPSTATE) + { + /* frame FP state is not fully filled, fill in the missing state from the current Unix side context. */ + frame->restore_flags &= ~RESTORE_FLAGS_INVALID_FPSTATE; + memset( &frame->xstate, 0, sizeof(frame->xstate) ); + if (user_shared_data->XState.CompactionEnabled) + frame->xstate.CompactionMask = 0x8000000000000000 | user_shared_data->XState.EnabledFeatures; + if (FPU_sig(ucontext)) + { + XSAVE_FORMAT xsave; + + xsave = *FPU_sig(ucontext); + memcpy( &xsave.XmmRegisters[6], &frame->xsave.XmmRegisters[6], 10 * sizeof(*xsave.XmmRegisters) ); + xsave.MxCsr = frame->xsave.MxCsr; + frame->xsave = xsave; + frame->xstate.Mask = XSTATE_MASK_LEGACY; + } + } NtGetContextThread( GetCurrentThread(), &context->c ); if (xstate_extended_features) { @@ -3145,7 +3165,7 @@ __ASM_GLOBAL_FUNC( __wine_unix_call_dispatcher, "popq 0x80(%rcx)\n\t" /* frame->eflags */ __ASM_CFI(".cfi_adjust_cfa_offset -8\n\t") __ASM_CFI_REG_IS_AT2(rip, rcx, 0xf0,0x00) - "movl $0,0xb4(%rcx)\n\t" /* frame->restore_flags */ + "movl $0x200000,0xb4(%rcx)\n\t" /* frame->restore_flags <- RESTORE_FLAGS_INVALID_FPSTATE */ __ASM_LOCAL_LABEL("__wine_unix_call_dispatcher_prolog_end") ":\n\t" "movq %rbx,0x08(%rcx)\n\t" __ASM_CFI_REG_IS_AT1(rbx, rcx, 0x08) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9060
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/signal_i386.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index d628d44272d..cbebc39b2c8 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -505,6 +505,8 @@ struct syscall_frame C_ASSERT( sizeof(struct syscall_frame) == 0x280 ); +#define RESTORE_FLAGS_INVALID_FPSTATE 0x00008000 + struct x86_thread_data { UINT fs; /* 1d4 TEB selector */ @@ -1859,12 +1861,14 @@ static BOOL handle_syscall_trap( ucontext_t *sigcontext, siginfo_t *siginfo ) extern void __wine_syscall_dispatcher_prolog_end(void); EIP_sig( sigcontext ) = (ULONG)__wine_syscall_dispatcher_prolog_end; + frame->restore_flags = LOWORD(CONTEXT_CONTROL); } else if ((void *)EIP_sig( sigcontext ) == __wine_unix_call_dispatcher) { extern void __wine_unix_call_dispatcher_prolog_end(void); EIP_sig( sigcontext ) = (ULONG)__wine_unix_call_dispatcher_prolog_end; + frame->restore_flags = LOWORD(CONTEXT_CONTROL | RESTORE_FLAGS_INVALID_FPSTATE); } else if (siginfo->si_code == 4 /* TRAP_HWBKPT */ && is_inside_syscall( ESP_sig(sigcontext) )) { @@ -1877,7 +1881,6 @@ static BOOL handle_syscall_trap( ucontext_t *sigcontext, siginfo_t *siginfo ) frame->eip = *(ULONG *)ESP_sig( sigcontext ); frame->eflags = EFL_sig(sigcontext); - frame->restore_flags = LOWORD(CONTEXT_CONTROL); ECX_sig( sigcontext ) = (ULONG)frame; ESP_sig( sigcontext ) += sizeof(ULONG); @@ -2135,6 +2138,29 @@ static void usr1_handler( int signal, siginfo_t *siginfo, void *sigcontext ) return; } context->c.ContextFlags = CONTEXT_FULL | CONTEXT_EXCEPTION_REQUEST; + if (frame->restore_flags & RESTORE_FLAGS_INVALID_FPSTATE) + { + /* frame FP state is not fully filled, fill in the missing state from the current Unix side context. */ + frame->restore_flags &= ~RESTORE_FLAGS_INVALID_FPSTATE; + memset( &frame->xstate, 0, sizeof(frame->xstate) ); + if (user_shared_data->XState.CompactionEnabled) + frame->xstate.CompactionMask = 0x8000000000000000 | user_shared_data->XState.EnabledFeatures; + if (FPUX_sig(ucontext)) + { + frame->u.xsave = *FPUX_sig(ucontext); + /* Clear register stack. */ + frame->u.xsave.TagWord = 0; + frame->u.xsave.StatusWord = 0; + frame->xstate.Mask = XSTATE_MASK_LEGACY; + } + else if (FPU_sig(ucontext)) + { + frame->u.fsave = *FPU_sig(ucontext); + /* Clear register stack. */ + frame->u.fsave.TagWord = 0xffffffff; + frame->u.fsave.StatusWord = 0xffff0000; + } + } NtGetContextThread( GetCurrentThread(), &context->c ); if (xstate_extended_features) { @@ -2697,7 +2723,7 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher_return, */ __ASM_GLOBAL_FUNC( __wine_unix_call_dispatcher, "movl %fs:0x218,%ecx\n\t" /* thread_data->syscall_frame */ - "movl $0,(%ecx)\n\t" /* frame->restore_flags */ + "movl $0x8000,(%ecx)\n\t" /* frame->restore_flags <- RESTORE_FLAGS_INVALID_FPSTATE */ "popl 0x08(%ecx)\n\t" /* frame->eip */ __ASM_CFI(".cfi_adjust_cfa_offset -4\n\t") "pushfl\n\t" @@ -2738,7 +2764,7 @@ __ASM_GLOBAL_FUNC( __wine_unix_call_dispatcher, __ASM_CFI(".cfi_offset %edi,-20\n\t") "call *(%eax,%edx,4)\n\t" "leal 16(%esp),%esp\n\t" - "testl $0xffff,(%esp)\n\t" /* frame->restore_flags */ + "testl $0x7fff,(%esp)\n\t" /* frame->restore_flags */ "jnz " __ASM_LOCAL_LABEL("__wine_syscall_dispatcher_return") "\n\t" "movl 0x08(%esp),%ecx\n\t" /* frame->eip */ /* switch to user stack */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9060
v2: - reset FPU stack and status on i386 when grabbing Unix side FP context. Even if sse is used on the Unix side the float return values are still passed as x87 stack top. It is perhaps possible to make it more accurate on i386 by storing mxcsr as on x64 and additionally x87 control word in unix_call_dispatcher, but I hope we can get away without? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9060#note_116898
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)