[Bug 16544] New: winmm: mixerOpen(): when CALLBACK_WINDOW flag given, NULL Callback is also valid

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

Summary: winmm: mixerOpen(): when CALLBACK_WINDOW flag given,
                    NULL Callback is also valid
           Product: Wine
           Version: 1.1.10
          Platform: PC-x86-64
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: winmm&mci
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Hello,

while revisiting a Battlefield 2 issue I came across another one...

BF2 voice setup (C:\Program Files\EA GAMES\Battlefield 2\BF2VoiceSetup.exe)
crashes when "Save Settings" button is clicked.
This happens from BF2 sub-installer (causing no harm) or when the voice setup
app is run stand alone.

--- snip ---
..
wine: Unhandled page fault on read access to 0x0000002c at address 0x408661
(thread 0044), starting debugger...
Unhandled exception: page fault on read access to 0x0000002c in 32-bit code
(0x00408661).
Register dump:
 CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b
 EIP:00408661 ESP:0033dadc EBP:0033dbc0 EFLAGS:00010216(   - 00      -RIAP1)
 EAX:00000000 EBX:00000111 ECX:00000000 EDX:00000002
 ESI:0033dadc EDI:0033dbc0
Stack dump:
0x0033dadc:  00000001 0033f60c 00000111 cccccccc
0x0033daec:  cccccccc cccccccc cccccccc cccccccc
0x0033dafc:  cccccccc cccccccc cccccccc cccccccc
0x0033db0c:  cccccccc cccccccc cccccccc cccccccc
0x0033db1c:  cccccccc cccccccc cccccccc cccccccc
0x0033db2c:  cccccccc cccccccc cccccccc cccccccc
Backtrace:
=>0 0x00408661 in bf2voicesetup (+0x8661) (0x0033dbc0)
  1 0x00403978 in bf2voicesetup (+0x3978) (0x0033dd8c)
  2 0x7c171915 in mfc71 (+0x31915) (0x0033ddbc)
  3 0x7c14db36 in mfc71 (+0xdb36) (0x0033dde0)
  4 0x7c175cd8 in mfc71 (+0x35cd8) (0x0033de30)
  5 0x7c175cf2 in mfc71 (+0x35cf2) (0x0033dec4)
..
--- snip ---

The crash location is only indirectly related to the problem, hence this was a
bit tricky to debug to the real cause.
The real problem seems to be a Wine's winmm.mixerOpen() handling fdwOpen flags
with CALLBACK_WINDOW when dwCallback is NULL.

Consider the following (WINEDEBUG=+winmm):

--- snip ---
0023:trace:winmm:DllMain 0x60380000 0x1 0x1
0023:trace:winmm:MMDRV_Init ()
..
0023:trace:winmm:MIXER_Open (0x425f0c, 0, 00000000, 00000000, 00010000)
..
--- snip ---

mixerOpen() with CALLBACK_WINDOW flags and NULL dwCallback results in
MMSYSERR_INVALPARAM.
Corresponding Wine code:

--- snip dlls/winmm/winmm.c:MIXER_Open ---

UINT  MIXER_Open(LPHMIXER lphMix, UINT uDeviceID, DWORD_PTR dwCallback,
                 DWORD_PTR dwInstance, DWORD fdwOpen, BOOL bFrom32) 
{
...
  switch (fdwOpen & CALLBACK_TYPEMASK) { 
..
  case CALLBACK_WINDOW:
        mod.dwInstance = dwCallback;
        if (!IsWindow((HWND)dwCallback))
            return MMSYSERR_INVALPARAM;
        break;
    } 
..
}
--- snip dlls/winmm/winmm.c:MIXER_Open ---

MSDN: http://msdn.microsoft.com/en-us/library/ms712134.aspx

It says: "The dwCallback parameter is assumed to be a window handle (HWND)."

Unfortunately the app expects MMSYSERR_NOERROR but not a failure (bad app error
handling anyway).
Hence crucial data structures are not getting setup properly (C++
instances/member data), leading to NULL ptr dereference at much later time.

A small conformance test case (fdwOpen=CALLBACK_WINDOW, dwCallback=NULL) should
reveal this MSDN documentation insufficiency.

Regards