[Bug 27393] New: mmdevapi uninitialiased memory access and crash past rendering

http://bugs.winehq.org/show_bug.cgi?id=27393

Summary: mmdevapi uninitialiased memory access and crash past
                    rendering
           Product: Wine
           Version: 1.3.21
          Platform: x86
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: mmdevapi
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Created an attachment (id=35024)
 --> (http://bugs.winehq.org/attachment.cgi?id=35024)
patch to mmdevapi/tests/render.c to play silence

Hi,

I've attached one version of the patch to mmdevapi/tests/render.c that works
well with testbot and seems to work well with Ubuntu Intrepid with PulseAudio,
but produces random crashes within a critical section when used with dmix.

mmdevdrv.d:837 NULL pointer access when looking for a GUID, called from
test_session i.e. after my changes in test_clock!

I found that very suspicious and thus ran the test in Ubuntu Lucid with
Valgrind (and PulseAudio), and it shows:

wine: Unhandled page fault on read access to 0xdededef6 at address 0x7bc34880
(thread 0020), starting debugger...
DEDEDE comes from a read after free() provoked by running

VALGRIND_OPTS="--trace-children=yes --log-file=/dev/shm/v%p.log
--leak-check=full --malloc-fill=A5 --free-fill=DE --track-origins=yes
--vex-iropt-precise-memory-exns=yes" WINETEST_WRAPPER=valgrind make render.ok

Thread 2:
Invalid read of size 4
   at 0x7BC34876: RtlpWaitForCriticalSection (critsection.c:434)
   by 0x7BC34F64: RtlEnterCriticalSection (critsection.c:546)
   by 0x541672A: alsa_push_buffer_data (mmdevdrv.c:1429)
   by 0x7BC7C53A: timer_callback_wrapper (threadpool.c:601)
   by 0x7BC7DA59: timer_queue_thread_proc (threadpool.c:670)
   by 0x7BC71737: ??? (in /wine-lucid/dlls/ntdll/ntdll.dll.so)
   by 0x7BC7190D: call_thread_entry_point (signal_i386.c:2499)
   by 0x7BC7B524: start_thread (thread.c:404)
   by 0x418296D: start_thread (pthread_create.c:300)
   by 0x4263A4D: clone (clone.S:130)
 Address 0x7f037dcc is not stack'd, malloc'd or (recently) free'd

Invalid read of size 4
   at 0x7BC34880: RtlpWaitForCriticalSection (critsection.c:434)
   by 0x7BC34F64: RtlEnterCriticalSection (critsection.c:546)
   by 0x541672A: alsa_push_buffer_data (mmdevdrv.c:1429)
   by 0x7BC7C53A: timer_callback_wrapper (threadpool.c:601)
   by 0x7BC7DA59: timer_queue_thread_proc (threadpool.c:670)
   by 0x7BC71737: ??? (in /wine-lucid/dlls/ntdll/ntdll.dll.so)
   by 0x7BC7190D: call_thread_entry_point (signal_i386.c:2499)
   by 0x7BC7B524: start_thread (thread.c:404)
   by 0x418296D: start_thread (pthread_create.c:300)
   by 0x4263A4D: clone (clone.S:130)
 Address 0xdededef6 is not stack'd, malloc'd or (recently) free'd

My render loop may not be bug free - please check - but I believe the illegal
access to be in mmdevapi.
I recommend adding a player loop to the tests.

There really is a timing issue.  Sometimes, even with Valgrind, there's no
illegal access during the test run.  In that case I typically see:
err:ntdll:RtlpWaitForCriticalSection section 0x7f41a704 "?" wait timed out in
thread 0022, blocked by 0023, retrying (60 sec)
Perhaps the critical section was destroyed, somewhat too early?

BTW, the CoTaskMemFree within the patch are part of a bug fix that I'll submit
this Monday.

[Bug 27393] mmdevapi uninitialiased memory access and crash past rendering

http://bugs.winehq.org/show_bug.cgi?id=27393

--- Comment #1 from Jörg Höhle  2011-06-05 03:05:23 CDT ---
On Mac OSX 10,5.8, the patch generates an exception in
AudioRenderClient_ReleaseBuffer. That may be bug #27184 (although I observe an
illegal read access to 0x00000004, not a write access as originally reported).

[Bug 27393] mmdevapi uninitialiased memory access and crash past rendering

http://bugs.winehq.org/show_bug.cgi?id=27393

--- Comment #2 from Jörg Höhle  2011-06-17 01:55:25 CDT ---
One major bug in winealsa is that AudioRenderClient_ReleaseBuffer uses
lcl_offs_frames to compute the buffer pointer supplied at GetBuffer time to the
application.
This is wrong because it is independently updated by the timer callback.  As a
result of using a wrong buffer position, memset writes past the buffer's end,
which is exactly what Wine detects:
001f:err:heap:HEAP_ValidateInUseArena Heap 0x110000: block 0x4cca60 tail
overwritten at 0x4fb860 (byte 0/8 == 0x00)

[Bug 27393] mmdevapi uninitialiased memory access and crash past rendering

http://bugs.winehq.org/show_bug.cgi?id=27393

--- Comment #3 from Jörg Höhle  2011-07-18 11:58:29 CDT ---
IMHO it was an error to transition winmm to mmdevapi before fixing this major
bug.

[Bug 27393] mmdevapi uninitialiased memory access and crash past rendering

http://bugs.winehq.org/show_bug.cgi?id=27393

Andrew Eikum  changed:

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

[Bug 27393] mmdevapi uninitialiased memory access and crash past rendering

http://bugs.winehq.org/show_bug.cgi?id=27393

--- Comment #4 from Andrew Eikum  2011-07-26 09:06:44 CDT ---
Created an attachment (id=35702)
 --> (http://bugs.winehq.org/attachment.cgi?id=35702)
winealsa.drv: Fix AudioRenderClient write pointer calculation

Thanks for the nice analysis, Jörg. I think the problem is calculating the
buffer write pointer incorrectly. If you look at the patch, you'll see two
spots where it calculates the write pointer offset using only lcl_offs_frames.
It should use (lcl_offs_frames + held_frames). This patch changes that.

I wasn't able to reproduce your crash here, unfortunately. So please give this
a try and see if it's any better.

[Bug 27393] mmdevapi uninitialiased memory access and crash past rendering

http://bugs.winehq.org/show_bug.cgi?id=27393

--- Comment #5 from Andrew Eikum  2011-07-27 13:28:03 CDT ---
Patch is in Wine now as 96aa86c99deeb12ff7c972370e2ca8fe4d1d8161. Please give
it a test and let me know what you think.

[Bug 27393] mmdevapi uninitialiased memory access and crash past rendering

http://bugs.winehq.org/show_bug.cgi?id=27393

Jörg Höhle  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |major

--- Comment #6 from Jörg Höhle  2011-07-28 02:49:36 CDT ---
Unfortunately, it's currently very hard for me to validate anything, because
the tandem mmdevapi + winmm is in still in too bad shape.

Using Ubuntu Intrepid with dmix (Pulseaudio killed)
render.ok passes even with WINEDEBUG=warn+heap
winmm:tests pass without warn+heap
With warn+heap, capture crashes, wave deadlocks in heap.c, mci crashes towards
the end.  There's no backtrace when crashing.  IIRC there are several errors
about WAVERRR_STILLPLAYING.

The interactive wave test (runtest -i) plays the sines correctly without
warn+heap.  That's a good start.
The tests produce several errors from winmm_waveopen: Activate failed (8000XXXX
= E_FAIL).  What can explain those?

With Pulseaudio in Intrepid, I get so many crashes that it's unusable, even
without warn+heap.
IIRC, the interactive wave test without warn+heap generates a short sine tone,
not 5 seconds.  The tests produce lots of WAVERR_STILLPLAYING.

Same in Ubuntu Lucid, where I can't disable Pulseaudio: I get so many hangs and
crashes that I can't reasonably run the tests, therefore I can't use Valgrind
either. (IIRC, Valgrind does not work with Wine in Intrepid, that's why I
switch to Lucid). So I switched back to Intrepid.

One repeatable way to produce "Activate failed" with the MCI in Intrepid is
open 11kulaw.wav alias u
open 11kadpcm.wav alias a
play a from 1000 to 2000
play u from 1000 to 2000
play u from 1000 to 2000

BTW, when I perform mmdevapi:render tests it always includes the "play silence"
from my attachment #35024.

[Bug 27393] mmdevapi uninitialiased memory access and crash past rendering

http://bugs.winehq.org/show_bug.cgi?id=27393

Jörg Höhle  changed:

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

--- Comment #7 from Jörg Höhle  2011-07-28 16:19:16 CDT ---
The crashes mentioned in comment #6 are gone from Ubuntu Intrepid. They were
caused by an unfortunate attempt of mine to work around the bug that
IsFormatSupported does not yet correctly handle pClosestMatch.

The tests now pass on Intrepid with both dmix and pulse and warn+heap.
Ubuntu Lucid and Valgrind will have to wait (mmdevapi/tests passes, mci still
hangs there, capture and wave show some failures in open).

Remaining issues in Intrepid:
- some occurrences of
err:winmm:WINMM_OpenDevice Activate failed: 80004005
- mci player still busy several seconds after audio ends normally, mentioning
xruns
... but that's another topic.

With Andrew's recent patch, mmdevapi can now play my render test
 => the present bug is fixed.

[Bug 27393] mmdevapi uninitialiased memory access and crash past rendering

http://bugs.winehq.org/show_bug.cgi?id=27393

Alexandre Julliard  changed:

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

--- Comment #8 from Alexandre Julliard  2011-08-05 12:38:49 CDT ---
Closing bugs fixed in 1.3.26.