dolphin
dolphin copied to clipboard
AudioCommon: Fix scratch buffer overflow
This would overwrite the std::FILE
pointer in IOFile
in WaveFileWriter
and cause a segfault in the IOFile
destructor when stopping a game.
I'm not familiar with audio code so maybe there is a better way to do it, but at least this fixes the crash.
Wouldn't it be simpler to limit available_samples
so it can't be bigger than MAX_SAMPLES
?
I don't think this would work in the case of MixSurround
.
Only fixed conflicts, no code changes.
I fixed the same issue in #10741 (446e4fbbb0156383ffffadaadf5300d54378c777), though I specifically fixed the broken bounds check in WaveFile so that no audio would be recorded in those broken cases (and separately changed the audio code so that the amount of samples written at any time is much smaller).
Out of curiosity, what games did you run into this issue in? The only ones I've seen it happen in are unlicensed titles by Datel.
On my Mario Kart Wii mod which makes extensive use of the /dev/dolphin
speed limit ioctl to speed up loading times (which made the bug so prevalent I think). But I haven't experienced the issue in a long time (something certainly changed either in Dolphin or in my own code but I have no idea what).
I guess more importantly, did you have audio dumping (Movie → Dump Audio) enabled when the crashes occurred? (Are there a bunch of files in Dolphin Emulator/Dump/Audio
?) If you didn't have it enabled, then that would have to be a separate overflow from the one I experienced and have a fix for (perhaps Mixer::m_scratch_buffer
overflowing instead of WaveFileWriter::conv_buffer
overflowing).
Audio dumping was not enabled.
I've created #11011 to add some assertions to hopefully make it more obvious when this overflow occurs. I don't think a resizable buffer is the right choice, but I'm also not sure when the overflow occurs in practice.
I trigger one of the asserts from #11011 attempting to load MKW-SP on Dolphin 5.0-17995 on Windows 10.