[PATCH] ntdll: Decouple pthread stack and win32 stack.

Jefferson Carpenter jeffersoncarpenter2 at gmail.com
Sun Aug 16 09:02:27 CDT 2020


Is there any particular reason to place the pthread stack adjacent to 
the win32 stack?  If the pthread stack happened to extend past 
PTHREAD_STACK_MIN while the win32 thread was started, writes to the 
win32 stack (such as the initial CONTEXT) would overwrite the pthread stack.

-Jefferson
-------------- next part --------------
From e89b057f9e454d67c9ccae37fc8368152afe754f Mon Sep 17 00:00:00 2001
From: Jefferson Carpenter <jeffersoncarpenter2 at gmail.com>
Date: Sun, 16 Aug 2020 15:58:30 +0000
Subject: [PATCH] ntdll: Decouple pthread stack and win32 stack.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49495
Signed-off-by: Jefferson Carpenter <jeffersoncarpenter2 at gmail.com>
---
 dlls/ntdll/unix/thread.c       | 17 ++++++++++++-----
 dlls/ntdll/unix/unix_private.h |  3 +--
 dlls/ntdll/unix/virtual.c      | 25 +++----------------------
 3 files changed, 16 insertions(+), 29 deletions(-)

diff --git a/dlls/ntdll/unix/thread.c b/dlls/ntdll/unix/thread.c
index cb7377017b..3356a3dd75 100644
--- a/dlls/ntdll/unix/thread.c
+++ b/dlls/ntdll/unix/thread.c
@@ -146,10 +146,11 @@ NTSTATUS WINAPI NtCreateThreadEx( HANDLE *handle, ACCESS_MASK access, OBJECT_ATT
     struct ntdll_thread_data *thread_data;
     DWORD tid = 0;
     int request_pipe[2];
-    SIZE_T extra_stack = PTHREAD_STACK_MIN;
+    SIZE_T pthread_stack_size = PTHREAD_STACK_MIN * 4;
     CLIENT_ID client_id;
     TEB *teb;
     INITIAL_TEB stack;
+    INITIAL_TEB pthread_stack;
     NTSTATUS status;
 
     if (process != NtCurrentProcess())
@@ -217,12 +218,19 @@ NTSTATUS WINAPI NtCreateThreadEx( HANDLE *handle, ACCESS_MASK access, OBJECT_ATT
 
     if ((status = virtual_alloc_teb( &teb ))) goto done;
 
-    if ((status = virtual_alloc_thread_stack( &stack, stack_reserve, stack_commit, &extra_stack )))
+    if ((status = virtual_alloc_thread_stack( &stack, stack_reserve, stack_commit )))
     {
         virtual_free_teb( teb );
         goto done;
     }
 
+    if ((status = virtual_alloc_thread_stack( &pthread_stack, pthread_stack_size, pthread_stack_size )))
+    {
+        NtFreeVirtualMemory( GetCurrentProcess(), &stack.StackBase, &stack_reserve, MEM_RELEASE );
+        virtual_free_teb( teb );
+        goto done;
+    }
+
     client_id.UniqueProcess = ULongToHandle( GetCurrentProcessId() );
     client_id.UniqueThread  = ULongToHandle( tid );
     teb->ClientId = client_id;
@@ -233,13 +241,12 @@ NTSTATUS WINAPI NtCreateThreadEx( HANDLE *handle, ACCESS_MASK access, OBJECT_ATT
 
     thread_data = (struct ntdll_thread_data *)&teb->GdiTebBatch;
     thread_data->request_fd  = request_pipe[1];
-    thread_data->start_stack = (char *)teb->Tib.StackBase;
+    thread_data->start_stack = pthread_stack.DeallocationStack;
     thread_data->start = start;
     thread_data->param = param;
 
     pthread_attr_init( &pthread_attr );
-    pthread_attr_setstack( &pthread_attr, teb->DeallocationStack,
-                           (char *)teb->Tib.StackBase + extra_stack - (char *)teb->DeallocationStack );
+    pthread_attr_setstack( &pthread_attr, pthread_stack.DeallocationStack, pthread_stack_size );
     pthread_attr_setguardsize( &pthread_attr, 0 );
     pthread_attr_setscope( &pthread_attr, PTHREAD_SCOPE_SYSTEM ); /* force creating a kernel thread */
     InterlockedIncrement( &nb_threads );
diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h
index 397211957b..21016a2445 100644
--- a/dlls/ntdll/unix/unix_private.h
+++ b/dlls/ntdll/unix/unix_private.h
@@ -204,8 +204,7 @@ extern TEB *virtual_alloc_first_teb(void) DECLSPEC_HIDDEN;
 extern NTSTATUS virtual_alloc_teb( TEB **ret_teb ) DECLSPEC_HIDDEN;
 extern void virtual_free_teb( TEB *teb ) DECLSPEC_HIDDEN;
 extern NTSTATUS virtual_clear_tls_index( ULONG index ) DECLSPEC_HIDDEN;
-extern NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, SIZE_T reserve_size, SIZE_T commit_size,
-                                            SIZE_T *pthread_size ) DECLSPEC_HIDDEN;
+extern NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, SIZE_T reserve_size, SIZE_T commit_size ) DECLSPEC_HIDDEN;
 extern void virtual_map_user_shared_data(void) DECLSPEC_HIDDEN;
 extern NTSTATUS virtual_handle_fault( void *addr, DWORD err, void *stack ) DECLSPEC_HIDDEN;
 extern unsigned int virtual_locked_server_call( void *req_ptr ) DECLSPEC_HIDDEN;
diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c
index a2fea58883..117acf3dc8 100644
--- a/dlls/ntdll/unix/virtual.c
+++ b/dlls/ntdll/unix/virtual.c
@@ -2751,13 +2751,12 @@ NTSTATUS virtual_clear_tls_index( ULONG index )
 /***********************************************************************
  *           virtual_alloc_thread_stack
  */
-NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, SIZE_T reserve_size, SIZE_T commit_size,
-                                     SIZE_T *pthread_size )
+NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, SIZE_T reserve_size, SIZE_T commit_size )
 {
     struct file_view *view;
     NTSTATUS status;
     sigset_t sigset;
-    SIZE_T size, extra_size = 0;
+    SIZE_T size;
 
     if (!reserve_size || !commit_size)
     {
@@ -2769,11 +2768,10 @@ NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, SIZE_T reserve_size, SI
     size = max( reserve_size, commit_size );
     if (size < 1024 * 1024) size = 1024 * 1024;  /* Xlib needs a large stack */
     size = (size + 0xffff) & ~0xffff;  /* round to 64K boundary */
-    if (pthread_size) *pthread_size = extra_size = max( page_size, ROUND_SIZE( 0, *pthread_size ));
 
     server_enter_uninterrupted_section( &virtual_mutex, &sigset );
 
-    if ((status = map_view( &view, NULL, size + extra_size, FALSE,
+    if ((status = map_view( &view, NULL, size, FALSE,
                             VPROT_READ | VPROT_WRITE | VPROT_COMMITTED, 0 )) != STATUS_SUCCESS)
         goto done;
 
@@ -2788,23 +2786,6 @@ NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, SIZE_T reserve_size, SI
     mprotect_range( view->base, 2 * page_size, 0, 0 );
     VIRTUAL_DEBUG_DUMP_VIEW( view );
 
-    if (extra_size)
-    {
-        struct file_view *extra_view;
-
-        /* shrink the first view and create a second one for the extra size */
-        /* this allows the app to free the stack without freeing the thread start portion */
-        view->size -= extra_size;
-        status = create_view( &extra_view, (char *)view->base + view->size, extra_size,
-                              VPROT_READ | VPROT_WRITE | VPROT_COMMITTED );
-        if (status != STATUS_SUCCESS)
-        {
-            view->size += extra_size;
-            delete_view( view );
-            goto done;
-        }
-    }
-
     /* note: limit is lower than base since the stack grows down */
     stack->OldStackBase = 0;
     stack->OldStackLimit = 0;
-- 
2.26.2



More information about the wine-devel mailing list