Bug tracked down in mci.c

[Erland Lewin]

Hello,

I just got the latest wine from cvs, and it crashed when I tried to run 
Half Life.
  The problem turned out to be in mci.c, in the mciSendStringA function.
  The variable lpCmd is set to point into the (in my case mciavi) 
driver's command table, at the word "open". However, towards the end of 
the mciSendStringA function, there is the following code:

if (strcmp(verb, "open") == 0)
    {
    if ((dwRet = MCI_FinishOpen(wmd, (LPMCI_OPEN_PARMSA)data, dwFlags)))
        MCI_UnLoadMciDriver(iData, wmd);
    /* FIXME: notification is not properly shared across two opens */
    } else {
    dwRet = MCI_SendCommand(wmd->wDeviceID, MCI_GetMessage(lpCmd), 
dwFlags, (DWORD)data, TRUE);
    }
    TRACE("=> 1/ %lx (%s)\n", dwRet, lpstrRet);
    dwRet = MCI_HandleReturnValues(iData, dwRet, wmd, lpCmd, data, 
lpstrRet, uRetLen);

The problem is that MCI_UnLoadMciDriver is called, which causes the 
driver to be removed from memory, which means that lpCmd points to 
invalid memory.
  When MCI_HandleReturnValues then tries to use its lpCmd parameter, a 
segmentation fault will occur, becuase it tries to read from unmapped 
memory.
  I did a quick hack to work around the problem (lpCmd = strdup( lpCmd 
)) before the MCI_FinishOpen. I'd appreciate it if someone else could 
add a better permanent fix...

Thanks for the good work on Wine!

[Eric Pouech]

Erland Lewin a écrit :
> Hello,
>
> I just got the latest wine from cvs, and it crashed when I tried to run
> Half Life.
>   The problem turned out to be in mci.c, in the mciSendStringA function.
>   The variable lpCmd is set to point into the (in my case mciavi)
> driver's command table, at the word "open". However, towards the end of
> the mciSendStringA function, there is the following code:

well spotted!
I think a better fix would be to pass to MCI_HandleReturnValues the
computed value of MCI_GetReturnType (we already use it a bit upper in
mciSendString). this would be better (computing only once the return
value).
I'll submit a patch later on if you wish (since my winmm code is
currently
under great rewrite, and a few patches are already pending on
Alexandre's
queue), that should take a couple of days to be sorted out

A+