[Bug 19124] New: The Westerner: dsound heap overflow prevented by warn+heap

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

Summary: The Westerner: dsound heap overflow prevented by
                    warn+heap
           Product: Wine
           Version: 1.1.24
          Platform: PC
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: directx-dsound
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Created an attachment (id=22087)
 --> (http://bugs.winehq.org/attachment.cgi?id=22087)
short backtrace

The Westerner crashes without WINEDEBUG=warn+heap in DSOUND_bufpos_to_mixpos()
100         DWORD ret = pos * 32 / device->pwfx->wBitsPerSample;

So either the application or dsound is guilty of heap corruption / producing
broken data.

With the memory fence installed by using WINEDEBUG=warn+heap, the application
seems to work "normally", except 2 lines are repeated very often and always in
pair:
err:dsound:DSOUND_MixInBuffer length not a multiple of block size, len = 5120,
block size = 64626
warn:heap:allocate_large_block Could not allocate block for fc720000 bytes

I tried out ALSA, emulation, 22050Hz 8bit, or default full HW (and also OSS
IIRC). No difference.
Note that in my test data from 2009-06-14, I required a native quartz.dll, but
one also gets this crash with pure Wine components, before quartz gets used.

Using Ubuntu 8.10 on Intel/SigmalTel/AC'97 audio HW.

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

--- Comment #1 from Jörg Höhle   2009-07-01 04:10:40 ---
Created an attachment (id=22126)
 --> (http://bugs.winehq.org/attachment.cgi?id=22126)
valgrind log

Valgrind confirms the illegal access in DSOUND_bufpos_to_mixpos
==9455== Conditional jump or move depends on uninitialised value(s)
==9455==    at 0x513F006: DSOUND_bufpos_to_mixpos (mixer.c:101)

but also shows that something is wrong already ealier, when scanning the codecs
(msg711, imaadp32 etc.)

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

--- Comment #2 from Jörg Höhle   2009-07-01 04:25:26 ---
Created an attachment (id=22127)
 --> (http://bugs.winehq.org/attachment.cgi?id=22127)
bzip2 WINEDEBUG=+all,-afew log (1MB, uncompressed 40MB)

Hopefully somebody acquainted with dsound will immediately spot what's going
wrong. The first exception is at 65% into the file and there's another one at
89%.

The log looses me. The bug could could be anything:
- 0047:warn:wave:wodOpen Bad format: tag=0001 nChannels=3 nSamplesPerSec=44100
!
- unsupported WODM_UNPREPARE;
- lack of error checking in the application when returned an error from Wine
that would typically not occur in MS-Windows;
- ...?
Still that does not explain why warn+heap allows the app to run.

The log was generated from
wine winemine
WINEDEBUG=+all,-syslevel,-fixup,-dbghelp_dwarf,-heap,warn-heap,-gdi wine
PICTuRE.exe 2>>w.log
so it's hopefully more specific to the application but may omit data from other
threads.

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

--- Comment #3 from Jörg Höhle   2009-09-15 11:00:33 ---
It's only now that I realize that bug #20056
"crash follows bogus DSOUND:SetFormat call"
completes the information here, but is not a different bug.
> From that we now know there's at least one bug in SetFormat()'s handling of the

device->pwfx structure. Why warn+heap matters is still unclear.

For whatever reason, The Westerner calls SetFormat initially (when displaying
the main menu) with the bogus values shown in bug #20056 (with or without
warn+heap).
(formattag=0x0001,chans=0,samplerate=0,bytespersec=0,blockalign=0,bitspersamp=0,cbSize=0)
but I've also seen chans=1812, nSamplesPerSec=456741109, nBlockAlign=28872

It's only when the introductory western movie starts that reasonable values are
supplied.

That bogus SetFormat explains all the rest. Those bogus values are stored in
device->pwfx.
After ReopenDevice, the timer interrupts kicks in but does do not much.
When CreateSoundBuffer is called, the bogus values lead RecalcFreqAcc to
estimate the new buffer length to nearly 2GB (which explains the errors from
allocate_large_block).
After that, when the timer interrupt kicks in, MixToTemporary wants to adjust
the frequency from 22050Hz to 456MHz. The error from MixInBuffer about length
not being a multiple of block size follows soon.
And again and again every time the interrupt triggers.

An experimental plausibility check (nchannels<6) aborting SetFormat leads to
reasonable behaviour. warn+heap is then not needed anymore.

http://msdn.microsoft.com/en-us/library/bb206059(VS.85).aspx
"The method succeeds even if the hardware does not support the requested
format; DirectSound sets the buffer to the closest supported format. To
determine whether this has happened, an application can call the GetFormat
method for the primary buffer and compare the result with the format that was
requested with the SetFormat method."
This suggests that it is acceptable to ignore bogus formats (that can be
detected early), arguing that the "closest supported" was the previous or
default one.  The app ought to call GetFormat to see what's actually being
used.

Nevertheless the dsound code should leave the device in a sane state even after
ReopenDevice fails.

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

--- Comment #4 from Jörg Höhle   2009-09-15 11:13:56 ---
*** Bug 20056 has been marked as a duplicate of this bug. ***

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

Dan Kegel  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
   Target Milestone|---                         |1.2.0

--- Comment #5 from Dan Kegel   2009-11-04 22:43:26 ---
Nice analysis, sound has been a pain -> nominating for 1.2

Jörg, think you could write a wine testcase for the
bogus SetFormat calls?  See http://www.winehq.org/docs/winedev-guide/testing

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

--- Comment #6 from Jörg Höhle   2009-11-05 07:47:38 ---
I myself find my analysis incomplete because I've never determined the origin
of the bogus SetFormat values. Does it originate from the app itself or does it
feed back into Wine bogus values it received from Wine (like garbage left from
the unimplemented amstream GetFormat call)?

On the Mac (not Linux), there's something really strange happening: with the
SetFormat plausibility checker in place, the screen becomes black every second
for a short while, much stronger than previously. The game is thus unplayable.
I've not looked into this since -- what has dsound to do with the screen?
> write a wine testcase for the bogus SetFormat?

I'll put that on my list (after the MCI stuff).
A fix to leave the device in a sane state even after ReopenDevice fails would
be much better though.

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

Maarten Lankhorst  changed:

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

--- Comment #7 from Maarten Lankhorst   2009-11-08 04:13:29 ---
Can you attach a WINEDEBUG=+dsound log only?

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

--- Comment #8 from Jörg Höhle   2009-11-09 15:34:51 ---
Created an attachment (id=24628)
 --> (http://bugs.winehq.org/attachment.cgi?id=24628)
+dsound log from Mac, Wine git as of 2009-11-07

Here's a log without warn+heap fence. Note that it crashes very early (after
the config menu, before any intro). The SetFormat with almost all zeroes is
still there and causing the known effects (reopen fails etc.)
With warn+heap, the Mac almost hangs, as mentioned previously.

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

Jörg Höhle  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #24628|application/octet-stream    |text/plain
          mime type|                            |

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

--- Comment #9 from Dan Kegel   2010-08-05 15:49:51 ---
Is this just an app bug, i.e. did it fail to initialize the struct it
passes to SetFormat, and it gets lucky on Windows because the
memory happens to be zeroed there?

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

--- Comment #10 from Jörg Höhle   2010-08-06 04:37:02 ---
I don't know where the bad format comes from.
However, it's also a Wine bug: Native refuses the bad format (alas I never got
along to writing testcases as you suggested in comment #5, the MCI keeps me
busy enough) and stays in (or recovers) a working state while Wine breaks
havoc.
Bug #16834 is another case where error paths in dsound cause it to become
internally inconsistent.

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

--- Comment #11 from Jörg Höhle   2010-08-11 05:23:45 ---
Bug #18823, comment #1 also reports bogus parameters to DSOUND_PrimarySetFormat
causing an app to crash in dsound.

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

Raymond  changed:

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

--- Comment #12 from Raymond  2010-09-26 22:36:07 CDT ---
(In reply to comment #10)
> I don't know where the bad format comes from.
> However, it's also a Wine bug: Native refuses the bad format (alas I never got
> along to writing testcases as you suggested in comment #5, the MCI keeps me
> busy enough) and stays in (or recovers) a working state while Wine breaks
> havoc.
> Bug #16834 is another case where error paths in dsound cause it to become
> internally inconsistent.

this bug can be easily reproduced when using "hw" device (i.e. set registry
"UseDirectHW" to "Y" with "dsound_test dsound" when your sound card does not
support mono or 11025Hz or 22050Hz

The current implemenation of SetFormat() function of winealsa.drv is unable to
set the buffer to the closest supported format

1) most of those hardware mixing sound cards support 8bit or 16bit, when the
application requested for 32bit , winealsa.drv does not set the format to the
closest format - 16bit

2) Most of the onboard sound card does not support mono or 8bits, when the
application request for mono, winealsa.drv does not set the format to the
closest format - stereo  or 16 bits

The major reason is winealsa.drv does not set the dwFlags of DSCAPS according
to the hardware capability of the sound card , it just assume all the sound
card use "plug" plugin to provide all the conversion

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

Jörg Höhle  changed:

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

--- Comment #13 from Jörg Höhle  2012-01-16 01:49:27 CST ---
Fixed as of wine-1.3.37 with both Linux/ALSA and MacOS. Now the SetFormat
checks are good enough for this app. WINEDEBUG=warn+heap does not make a
difference anymore.

Got a glVertex3f crash on MacOS 10.5.8 once, though.

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

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

Alexandre Julliard  changed:

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

--- Comment #14 from Alexandre Julliard  2012-01-27 14:17:48 CST ---
Closing bugs fixed in 1.4-rc1.

[Bug 19124] The Westerner: dsound heap overflow prevented by warn+heap

[WineHQ Bugzilla]

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

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download
                URL|                            |https://web.archive.org/web
                   |                            |/20210226091558/http://dl.4
                   |                            |players.de/f1/pc/westerner/
                   |                            |demo_westerner_de.zip
                 CC|                            |[e-mail]