[PATCH v3 0/4] MR9060: ntdll: Fix crashes in syscall dispatcher due to attempt to restore unitialized data.
-- v3: 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 | 64 ++++++++++++++++++++++++++++++----- 1 file changed, 56 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index d628d44272d..16b0103c839 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 */ @@ -716,7 +718,7 @@ static inline void *init_handler( const ucontext_t *sigcontext ) * * Save the thread FPU context. */ -static inline void save_fpu( CONTEXT *context ) +static inline void save_fpu( I386_FLOATING_SAVE_AREA *fsave ) { struct { @@ -730,11 +732,10 @@ static inline void save_fpu( CONTEXT *context ) } float_status; - context->ContextFlags |= CONTEXT_FLOATING_POINT; - __asm__ __volatile__( "fnsave %0; fwait" : "=m" (context->FloatSave) ); + __asm__ __volatile__( "fnsave %0; fwait" : "=m" (*fsave) ); /* Reset unmasked exceptions status to avoid firing an exception. */ - memcpy(&float_status, &context->FloatSave, sizeof(float_status)); + memcpy(&float_status, fsave, sizeof(float_status)); float_status.StatusWord &= float_status.ControlWord | 0xffffff80; __asm__ __volatile__( "fldenv %0" : : "m" (float_status) ); @@ -805,7 +806,11 @@ static inline void save_context( struct xcontext *xcontext, const ucontext_t *si if (!fpu) fpux_to_fpu( &context->FloatSave, fpux ); if (xstate_extended_features && (xs = XState_sig(fpux))) context_init_xstate( context, xs ); } - if (!fpu && !fpux) save_fpu( context ); + if (!fpu && !fpux) + { + save_fpu( &context->FloatSave ); + context->ContextFlags |= CONTEXT_FLOATING_POINT; + } } @@ -1859,12 +1864,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 +1884,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 +2141,48 @@ 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)) + { + if (user_shared_data->ProcessorFeatures[PF_XMMI_INSTRUCTIONS_AVAILABLE]) + frame->u.xsave = *FPUX_sig(ucontext); + else + fpux_to_fpu( &frame->u.fsave, FPUX_sig(ucontext) ); + frame->xstate.Mask = XSTATE_MASK_LEGACY; + } + else + { + I386_FLOATING_SAVE_AREA *fsave, fsave_buf; + + if (FPU_sig(ucontext)) fsave = FPU_sig(ucontext); + else + { + save_fpu( &fsave_buf ); + fsave = &fsave_buf; + } + if (user_shared_data->ProcessorFeatures[PF_XMMI_INSTRUCTIONS_AVAILABLE]) + fpu_to_fpux( &frame->u.xsave, fsave ); + else + frame->u.fsave = *fsave; + } + /* Clear register stack. */ + if (user_shared_data->ProcessorFeatures[PF_XMMI_INSTRUCTIONS_AVAILABLE]) + { + frame->u.fsave.TagWord = 0xffffffff; + frame->u.fsave.StatusWord = 0xffff0000; + } + else + { + frame->u.xsave.TagWord = 0; + frame->u.xsave.StatusWord = 0; + } + } NtGetContextThread( GetCurrentThread(), &context->c ); if (xstate_extended_features) { @@ -2697,7 +2745,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 +2786,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
v3: - better handle different tupe of states availability on i386. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9060#note_117029
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)