From: Rémi Bernon rbernon@codeweavers.com
Using a dedicated exit jmpbuf and removing the need for assembly routines.
When Wine handles an exception in unix code, we return to user mode by jumping to the last syscall frame. This can leave some pthread cancel cleanups registered, in the pthread internal linked list, and at the same time later overwrite the stack frame they were registered for.
In the same way, jumping to the exit frame on thread exit or abort, can also leave some cleanup handlers registered for invalid stack frames.
Depending on the implementation, calling pthread_exit will cause all the registered pthread cleanup handlers to be called, possibly jumping back to now overwritten stack frames and causing segmentation faults.
Exiting a pthread normally, by returning from its procedure, or calling exit(0) for the main thread doesn't run pthread_exit and doesn't call cleanup handlers, avoiding that situation.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 --- dlls/ntdll/unix/loader.c | 1 + dlls/ntdll/unix/server.c | 2 +- dlls/ntdll/unix/signal_arm.c | 13 ------------ dlls/ntdll/unix/signal_arm64.c | 12 ----------- dlls/ntdll/unix/signal_i386.c | 25 ----------------------- dlls/ntdll/unix/signal_x86_64.c | 21 -------------------- dlls/ntdll/unix/thread.c | 35 +++++++++++++++++++++++++-------- dlls/ntdll/unix/unix_private.h | 4 +++- 8 files changed, 32 insertions(+), 81 deletions(-)
diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index c21d32ea811..265a214d1fb 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -2510,4 +2510,5 @@ void __wine_main( int argc, char *argv[], char *envp[] ) apple_main_thread(); #endif start_main_thread(); + exit( 0 ); } diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index 77e8d5c7566..98afc42ad27 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -1581,7 +1581,7 @@ void server_init_process_done(void) SERVER_END_REQ;
assert( !status ); - signal_start_thread( entry, peb, suspend, NtCurrentTeb() ); + thread_start( entry, peb, suspend, NtCurrentTeb() ); }
diff --git a/dlls/ntdll/unix/signal_arm.c b/dlls/ntdll/unix/signal_arm.c index 5d1478a1ff4..ab00a67d436 100644 --- a/dlls/ntdll/unix/signal_arm.c +++ b/dlls/ntdll/unix/signal_arm.c @@ -1183,19 +1183,6 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "bl " __ASM_NAME("call_init_thunk") )
-/*********************************************************************** - * signal_exit_thread - */ -__ASM_GLOBAL_FUNC( signal_exit_thread, - "ldr r3, [r2, #0x1d4]\n\t" /* arm_thread_data()->exit_frame */ - "mov ip, #0\n\t" - "str ip, [r2, #0x1d4]\n\t" - "cmp r3, ip\n\t" - "it ne\n\t" - "movne sp, r3\n\t" - "blx r1" ) - - /*********************************************************************** * __wine_syscall_dispatcher */ diff --git a/dlls/ntdll/unix/signal_arm64.c b/dlls/ntdll/unix/signal_arm64.c index 4e552e0f10a..b24dbc2d8a4 100644 --- a/dlls/ntdll/unix/signal_arm64.c +++ b/dlls/ntdll/unix/signal_arm64.c @@ -1253,18 +1253,6 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "1:\tmov sp, x8\n\t" "bl " __ASM_NAME("call_init_thunk") )
-/*********************************************************************** - * signal_exit_thread - */ -__ASM_GLOBAL_FUNC( signal_exit_thread, - "stp x29, x30, [sp,#-16]!\n\t" - "ldr x3, [x2, #0x2f0]\n\t" /* arm64_thread_data()->exit_frame */ - "str xzr, [x2, #0x2f0]\n\t" - "cbz x3, 1f\n\t" - "mov sp, x3\n" - "1:\tldp x29, x30, [sp], #16\n\t" - "br x1" ) -
/*********************************************************************** * __wine_syscall_dispatcher diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 2dfce706394..0eecb5fb57e 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2468,31 +2468,6 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "call " __ASM_NAME("call_init_thunk") )
-/*********************************************************************** - * signal_exit_thread - */ -__ASM_GLOBAL_FUNC( signal_exit_thread, - "movl 8(%esp),%ecx\n\t" - "movl 12(%esp),%esi\n\t" - "xorl %edx,%edx\n\t" - /* fetch exit frame */ - "xchgl %edx,0x1f4(%esi)\n\t" /* x86_thread_data()->exit_frame */ - "testl %edx,%edx\n\t" - "jnz 1f\n\t" - "jmp *%ecx\n\t" - /* switch to exit frame stack */ - "1:\tmovl 4(%esp),%eax\n\t" - "movl %edx,%ebp\n\t" - __ASM_CFI(".cfi_def_cfa %ebp,4\n\t") - __ASM_CFI(".cfi_rel_offset %ebp,0\n\t") - __ASM_CFI(".cfi_rel_offset %ebx,-4\n\t") - __ASM_CFI(".cfi_rel_offset %esi,-8\n\t") - __ASM_CFI(".cfi_rel_offset %edi,-12\n\t") - "leal -20(%ebp),%esp\n\t" - "pushl %eax\n\t" - "call *%ecx" ) - - /*********************************************************************** * __wine_syscall_dispatcher */ diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 88c517daecf..fc73925e819 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -3324,27 +3324,6 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "call " __ASM_NAME("call_init_thunk"))
-/*********************************************************************** - * signal_exit_thread - */ -__ASM_GLOBAL_FUNC( signal_exit_thread, - /* fetch exit frame */ - "xorl %ecx,%ecx\n\t" - "xchgq %rcx,0x320(%rdx)\n\t" /* amd64_thread_data()->exit_frame */ - "testq %rcx,%rcx\n\t" - "jnz 1f\n\t" - "jmp *%rsi\n" - /* switch to exit frame stack */ - "1:\tmovq %rcx,%rsp\n\t" - __ASM_CFI(".cfi_adjust_cfa_offset 56\n\t") - __ASM_CFI(".cfi_rel_offset %rbp,48\n\t") - __ASM_CFI(".cfi_rel_offset %rbx,40\n\t") - __ASM_CFI(".cfi_rel_offset %r12,32\n\t") - __ASM_CFI(".cfi_rel_offset %r13,24\n\t") - __ASM_CFI(".cfi_rel_offset %r14,16\n\t") - __ASM_CFI(".cfi_rel_offset %r15,8\n\t") - "call *%rsi" ) - /*********************************************************************** * __wine_syscall_dispatcher */ diff --git a/dlls/ntdll/unix/thread.c b/dlls/ntdll/unix/thread.c index bb8c1c2936b..ff994ea1503 100644 --- a/dlls/ntdll/unix/thread.c +++ b/dlls/ntdll/unix/thread.c @@ -1050,24 +1050,43 @@ static void pthread_exit_wrapper( int status ) close( ntdll_get_thread_data()->wait_fd[1] ); close( ntdll_get_thread_data()->reply_fd ); close( ntdll_get_thread_data()->request_fd ); - pthread_exit( UIntToPtr(status) ); +} + +void DECLSPEC_NORETURN thread_exit( int status, void (*func)(int), TEB *teb ) +{ + struct ntdll_thread_data *thread_data = (struct ntdll_thread_data *)&teb->GdiTebBatch; + + func(status); + __wine_longjmp( thread_data->exit_buf, 1 ); }
/*********************************************************************** - * start_thread + * pthread_start_wrapper * * Startup routine for a newly created thread. */ -static void start_thread( TEB *teb ) +static void *pthread_start_wrapper( void *arg ) { + TEB *teb = arg; struct ntdll_thread_data *thread_data = (struct ntdll_thread_data *)&teb->GdiTebBatch; BOOL suspend;
thread_data->pthread_id = pthread_self(); signal_init_thread( teb ); server_init_thread( thread_data->start, &suspend ); - signal_start_thread( thread_data->start, thread_data->param, suspend, teb ); + thread_start( thread_data->start, thread_data->param, suspend, teb ); + return NULL; +} + +void thread_start( PRTL_THREAD_START_ROUTINE entry, void *arg, + BOOL suspend, TEB *teb ) +{ + struct ntdll_thread_data *thread_data = (struct ntdll_thread_data *)&teb->GdiTebBatch; + __wine_jmp_buf exit_buf; + + if (!__wine_setjmpex( (thread_data->exit_buf = &exit_buf), NULL )) + signal_start_thread( entry, arg, suspend, teb ); }
@@ -1345,7 +1364,7 @@ NTSTATUS WINAPI NtCreateThreadEx( HANDLE *handle, ACCESS_MASK access, OBJECT_ATT pthread_attr_setguardsize( &pthread_attr, 0 ); pthread_attr_setscope( &pthread_attr, PTHREAD_SCOPE_SYSTEM ); /* force creating a kernel thread */ InterlockedIncrement( &nb_threads ); - if (pthread_create( &pthread_id, &pthread_attr, (void * (*)(void *))start_thread, teb )) + if (pthread_create( &pthread_id, &pthread_attr, pthread_start_wrapper, teb )) { InterlockedDecrement( &nb_threads ); virtual_free_teb( teb ); @@ -1373,7 +1392,7 @@ void abort_thread( int status ) { pthread_sigmask( SIG_BLOCK, &server_block_set, NULL ); if (InterlockedDecrement( &nb_threads ) <= 0) abort_process( status ); - signal_exit_thread( status, pthread_exit_wrapper, NtCurrentTeb() ); + thread_exit( status, pthread_exit_wrapper, NtCurrentTeb() ); }
@@ -1406,7 +1425,7 @@ static DECLSPEC_NORETURN void exit_thread( int status ) virtual_free_teb( teb ); } } - signal_exit_thread( status, pthread_exit_wrapper, NtCurrentTeb() ); + thread_exit( status, pthread_exit_wrapper, NtCurrentTeb() ); }
@@ -1416,7 +1435,7 @@ static DECLSPEC_NORETURN void exit_thread( int status ) void exit_process( int status ) { pthread_sigmask( SIG_BLOCK, &server_block_set, NULL ); - signal_exit_thread( get_unix_exit_code( status ), process_exit_wrapper, NtCurrentTeb() ); + thread_exit( get_unix_exit_code( status ), process_exit_wrapper, NtCurrentTeb() ); }
diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 47f0f9c56a9..77d5feb7661 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -61,6 +61,7 @@ struct ntdll_thread_data PRTL_THREAD_START_ROUTINE start; /* thread entry point */ void *param; /* thread entry point parameter */ void *jmp_buf; /* setjmp buffer for exception handling */ + void *exit_buf; /* setjmp buffer for thread exit */ };
C_ASSERT( sizeof(struct ntdll_thread_data) <= sizeof(((TEB *)0)->GdiTebBatch) ); @@ -237,7 +238,8 @@ extern void signal_init_thread( TEB *teb ) DECLSPEC_HIDDEN; extern void signal_init_process(void) DECLSPEC_HIDDEN; extern void DECLSPEC_NORETURN signal_start_thread( PRTL_THREAD_START_ROUTINE entry, void *arg, BOOL suspend, TEB *teb ) DECLSPEC_HIDDEN; -extern void DECLSPEC_NORETURN signal_exit_thread( int status, void (*func)(int), TEB *teb ) DECLSPEC_HIDDEN; +extern void thread_start( PRTL_THREAD_START_ROUTINE entry, void *arg, + BOOL suspend, TEB *teb ) DECLSPEC_HIDDEN; extern SYSTEM_SERVICE_TABLE KeServiceDescriptorTable[4] DECLSPEC_HIDDEN; extern void __wine_syscall_dispatcher(void) DECLSPEC_HIDDEN; extern void WINAPI DECLSPEC_NORETURN __wine_syscall_dispatcher_return( void *frame, ULONG_PTR retval ) DECLSPEC_HIDDEN;