[Bug 53556] New: winealsa.drv segfaults on process exit

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53556

Bug ID: 53556
           Summary: winealsa.drv segfaults on process exit
           Product: Wine
           Version: 7.15
          Hardware: x86-64
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: ntdll
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

Created attachment 72919
  --> https://bugs.winehq.org/attachment.cgi?id=72919
Adds test to kernelbase_test.exe sync

I get a segfault on process exit whenever the winealsa.drv is loaded.

I can recreate this by running the winmm_test.exe midi test for example.

I've also attached a patch that adds a test to kernelbase_test.exe (under the
'sync' tests) that recreates this issue. It's recreated in this patch without
the process exit (thread termination is enough).

Here's the stacktrace for the winealsa.drv thread at the beginning of
signal_exit_thread (not long after SIGQUIT is signaled):
#0  0x00007ffff7cee3ce in signal_exit_thread ()
#1  0x00007ffff7d0d25b in abort_thread (status=status@entry=0) at
../../include/winnt.h:2246
#2  0x00007ffff7cf12e8 in quit_handler (signal=<optimised out>,
siginfo=<optimised out>, ucontext=<optimised out>) at
../../dlls/ntdll/unix/signal_x86_64.c:2861
#3  <signal handler called>
#4  0x00007ffff7e0a195 in __futex_abstimed_wait_common64 (private=0,
cancel=true, abstime=0x0, op=393, expected=0, futex_word=0x7ffff5aa2528
<notify_read_cond+40>) at ./nptl/futex-internal.c:57
#5  __futex_abstimed_wait_common (cancel=true, private=0, abstime=0x0,
clockid=0, expected=0, futex_word=0x7ffff5aa2528 <notify_read_cond+40>) at
./nptl/futex-internal.c:87
#6  __GI___futex_abstimed_wait_cancelable64
(futex_word=futex_word@entry=0x7ffff5aa2528 <notify_read_cond+40>,
expected=expected@entry=0, clockid=clockid@entry=0, abstime=abstime@entry=0x0, 
    private=private@entry=0) at ./nptl/futex-internal.c:139
#7  0x00007ffff7e0cac1 in __pthread_cond_wait_common (abstime=0x0, clockid=0,
mutex=0x7ffff5aa2540 <notify_mutex>, cond=0x7ffff5aa2500 <notify_read_cond>) at
./nptl/pthread_cond_wait.c:503
#8  ___pthread_cond_wait (cond=cond@entry=0x7ffff5aa2500 <notify_read_cond>,
mutex=mutex@entry=0x7ffff5aa2540 <notify_mutex>) at
./nptl/pthread_cond_wait.c:627
#9  0x00007ffff5a99a67 in midi_notify_wait (args=0x23cfdd0) at
../../dlls/winealsa.drv/alsamidi.c:1512
#10 0x00007ffff7cd7f13 in __wine_unix_call (handle=<optimised out>,
code=<optimised out>, args=0x7ffff5aa2528 <notify_read_cond+40>) at
../../dlls/ntdll/unix/loader.c:1362
#11 0x00007ffff7cee565 in __wine_syscall_dispatcher ()
#12 0x00000000024cf640 in ?? ()
#13 0x0000000000000019 in ?? ()
#14 0x00007ffff7e0d850 in ?? () at ./nptl/pthread_create.c:321
#15 0x00007ffff7cee3cc in signal_start_thread ()
#16 0x0000000000000000 in ?? ()

Before the call to __GI___futex_abstimed_wait_cancelable64 (made from frame 7),
the following line of code is executed:
https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_cond_wait.c;h=...

__pthread_cleanup_push (&buffer, __condvar_cleanup_waiting, &cbuffer);

This appears to push some cleanup code on to a cleanup stack that glibc later
attempts to use during thread termination. However, the buffer and cbuffer
structs are allocated on the threads stack. Exploring frame 7, we can get the
memory address for the buffer struct (0x24ce870):
(gdb) f 7
#7  0x00007ffff7e0cac1 in __pthread_cond_wait_common (abstime=0x0, clockid=0,
mutex=0x7ffff5aa2540 <notify_mutex>, cond=0x7ffff5aa2500 <notify_read_cond>) at
./nptl/pthread_cond_wait.c:503
warning: Source file is more recent than executable.
503          err = __futex_abstimed_wait_cancelable64 (
(gdb) p &buffer
$1 = (struct _pthread_cleanup_buffer *) 0x24ce870

I then step forward 4 instructions:
(gdb) si
0x00007ffff7cee3d5 in signal_exit_thread ()
1: x/i $rip
=> 0x7ffff7cee3d5 <signal_exit_thread+9>:    test   %rcx,%rcx
(gdb) 
0x00007ffff7cee3d8 in signal_exit_thread ()
1: x/i $rip
=> 0x7ffff7cee3d8 <signal_exit_thread+12>:    jne    0x7ffff7cee3dc
<signal_exit_thread+16>
(gdb) 
0x00007ffff7cee3dc in signal_exit_thread ()
1: x/i $rip
=> 0x7ffff7cee3dc <signal_exit_thread+16>:    mov    %rcx,%rsp
(gdb) 
0x00007ffff7cee3df in signal_exit_thread ()
1: x/i $rip
=> 0x7ffff7cee3df <signal_exit_thread+19>:    call   *%rsi
(gdb) bt
#0  0x00007ffff7cee3df in signal_exit_thread ()
#1  0x00007ffff7d0a1f0 in start_thread (teb=0x67fd0000) at
../../dlls/ntdll/unix/thread.c:1070
#2  0x00007ffff7e0db43 in start_thread (arg=<optimised out>) at
./nptl/pthread_create.c:442
#3  0x00007ffff7e9fa00 in clone3 () at
../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
(gdb) p $rsp
$2 = (void *) 0x24cee00

And now the $rsp register is updated (to 0x24cee00), but it's to the right of
the buffer struct. 0x24cee00 - 0x24ce870 = 0x590 (1424 bytes). So once the
stack expands 1424 bytes (to the left), this buffer struct is overwritten.

I add a watch to the buffer structs memory address and continue:
(gdb) watch ((struct _pthread_cleanup_buffer *) 0x24ce870)->__routine
Hardware watchpoint 3: ((struct _pthread_cleanup_buffer *)
0x24ce870)->__routine
(gdb) c
Continuing.

Thread 2 "winmm_test.exe" hit Hardware watchpoint 3: ((struct
_pthread_cleanup_buffer *) 0x24ce870)->__routine

Old value = (void (*)(void *)) 0x7ffff7e0c7a0 <__condvar_cleanup_waiting>
New value = (void (*)(void *)) 0x7ffff7e16330 <unwind_stop>
0x00007ffff7fc2426 in __GI__dl_find_object (pc1=0x7ffff51663e9
<_Unwind_ForcedUnwind+57>, result=0x24ce8d0) at ./elf/dl-find_object.c:363
363      if (__glibc_unlikely (_dlfo_main.map_end == 0))
1: x/i $rip
=> 0x7ffff7fc2426 <__GI__dl_find_object+6>:    push   %r13
(gdb) 
Continuing.

Thread 2 "winmm_test.exe" hit Hardware watchpoint 3: ((struct
_pthread_cleanup_buffer *) 0x24ce870)->__routine

Old value = (void (*)(void *)) 0x7ffff7e16330 <unwind_stop>
New value = (void (*)(void *)) 0x7ffff51663e9 <_Unwind_ForcedUnwind+57>
0x00007ffff5166c91 in get_cie_encoding (cie=0x7ffff516a1b8) at
../../../src/libgcc/unwind-dw2-fde.c:299
299      aug = cie->augmentation;
1: x/i $rip
=> 0x7ffff5166c91 <get_cie_encoding+1>:    mov    %rdi,%rbp
(gdb) 
Continuing.

Thread 2 "winmm_test.exe" hit Hardware watchpoint 3: ((struct
_pthread_cleanup_buffer *) 0x24ce870)->__routine

Old value = (void (*)(void *)) 0x7ffff51663e9 <_Unwind_ForcedUnwind+57>
New value = (void (*)(void *)) 0x24ce890
0x00007ffff516584c in uw_update_context_1 (context=0x24ceb90, fs=0x24ce9d0) at
../../../src/libgcc/unwind-dw2.c:1454
1454        switch (fs->regs.reg[i].how)
1: x/i $rip
=> 0x7ffff516584c <uw_update_context_1+492>:    lea    0x4209(%rip),%rbp       
# 0x7ffff5169a5c
(gdb) c
Continuing.

It actually gets updated a few times, but the last update is to an invalid
executable address (0x24ce890); which ultimately leads to the segfault:
Thread 2 "winmm_test.exe" received signal SIGSEGV, Segmentation fault.
0x00000000024ce890 in ?? ()
1: x/i $rip
=> 0x24ce890:    rex.WXB add %al,(%r8)
1: x/i $rip
=> 0x24ce890:    rex.WXB add %al,(%r8)

[Bug 53556] winealsa.drv segfaults on process exit

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53556

--- Comment #1 from Brendan McGrath  ---
Created attachment 72920
  --> https://bugs.winehq.org/attachment.cgi?id=72920
Experimental Fix

I've played with an experimental fix (attached) which does not move the $rsp
register, but does try to keep the backtrace within the CFI consistent.

This does fix the segfault, but I'm not sure what it breaks. It also only fixes
the x86_64 instruction set.

I also had to remove a call to exit_process in NtTerminateProcess, otherwise it
seems the libraries were being removed prior to the completion of the thread
termination (causing the return code of 56?). So whilst I needed this for the
x86_64 instruction set, this change will likely negatively impact the other
instruction sets.

[Bug 53556] winealsa.drv segfaults on process exit

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53556

Brendan McGrath  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #72920|0                           |1
        is obsolete|                            |

--- Comment #2 from Brendan McGrath  ---
Created attachment 75403
  --> https://bugs.winehq.org/attachment.cgi?id=75403
Experimental Fix 2

Adding a second experimental fix, which _I think_, is slightly less horrible

[Bug 53556] winealsa.drv segfaults on process exit

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53556

Fabian Maurer  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 53556] winealsa.drv segfaults on process exit

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53556

Brendan McGrath  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |DUPLICATE

--- Comment #3 from Brendan McGrath  ---
This appears to be a duplicate

*** This bug has been marked as a duplicate of bug 52213 ***

[Bug 53556] winealsa.drv segfaults on process exit

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53556

Alistair Leslie-Hughes  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #4 from Alistair Leslie-Hughes  ---
Closing duplicate.