[PATCH 0/1] MR2468: ntdll: Don't report user (PE) stack via pthread_attr_setstack().
Today, NtCreateThreadEx() passes to pthread_attr_setstack() an address range that spans both the user (PE) stack and the kernel (Unix) stack. pthread_attr_setstack() accepts an address range that will be used as the initial stack area for the thread created by pthread_create(). It is often assumed that the initial stack will be available for the entire duration of the thread's lifetime. This assumption, however, conflicts with how Win32 fibers operate. Fiber APIs allow the thread's initial stack to be freed before the thread exits, or kept alive beyond the point of thread's termination. This allows the lifetime of the thread's initial stack to be shorter or longer than the originating thread's lifetime. This is possible because each fiber has its own stack and context, and ConvertThreadToFiber() transfers the current thread's stack to a new fiber. This specifically causes problems in Glibc v2.31 and earlier. These Glibc versions have a bug where madvise(2) with the MADV_DONTNEED flag is called on the initial stack area on thread exit, even when the stack was user-supplied (via pthread_attr_setstack). Therefore, the kernel may zero out any portion of the initial stack at any time after the originating thread terminates, even if the stack no longer belongs to the current thread (either freed and reallocated, or owned by a fiber). This may ultimately lead to memory corruption. Fix this by only passing the syscall (kernel) portion of the stack to pthread_attr_setstack(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2468
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Today, NtCreateThreadEx() passes to pthread_attr_setstack() an address range that spans both the user (PE) stack and the kernel (Unix) stack. pthread_attr_setstack() accepts an address range that will be used as the initial stack area for the thread created by pthread_create(). It is often assumed that the initial stack will be available for the entire duration of the thread's lifetime. This assumption, however, conflicts with how Win32 fibers operate. Fiber APIs allow the thread's initial stack to be freed before the thread exits, or kept alive beyond the point of thread's termination. This allows the lifetime of the thread's initial stack to be shorter or longer than the originating thread's lifetime. This is possible because each fiber has its own stack and context, and ConvertThreadToFiber() transfers the current thread's stack to a new fiber. This specifically causes problems in Glibc v2.31 and earlier. These Glibc versions have a bug where madvise(2) with the MADV_DONTNEED flag is called on the initial stack area on thread exit, even when the stack was user-supplied (via pthread_attr_setstack). Therefore, the kernel may zero out any portion of the initial stack at any time after the originating thread terminates, even if the stack no longer belongs to the current thread (either freed and reallocated, or owned by a fiber). This may ultimately lead to memory corruption. Fix this by only passing the syscall (kernel) portion of the stack to pthread_attr_setstack(). --- dlls/ntdll/unix/thread.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dlls/ntdll/unix/thread.c b/dlls/ntdll/unix/thread.c index 3228180e20f..817d152346f 100644 --- a/dlls/ntdll/unix/thread.c +++ b/dlls/ntdll/unix/thread.c @@ -1374,8 +1374,7 @@ NTSTATUS WINAPI NtCreateThreadEx( HANDLE *handle, ACCESS_MASK access, OBJECT_ATT thread_data->param = param; pthread_attr_init( &pthread_attr ); - pthread_attr_setstack( &pthread_attr, teb->DeallocationStack, - (char *)thread_data->kernel_stack + kernel_stack_size - (char *)teb->DeallocationStack ); + pthread_attr_setstack( &pthread_attr, thread_data->kernel_stack, kernel_stack_size ); pthread_attr_setguardsize( &pthread_attr, 0 ); pthread_attr_setscope( &pthread_attr, PTHREAD_SCOPE_SYSTEM ); /* force creating a kernel thread */ InterlockedIncrement( &nb_threads ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2468
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2468
participants (3)
-
Jacek Caban (＠jacek) -
Jinoh Kang
-
Jinoh Kang (＠iamahuman)