LASTEXPRESS: fix race condition in sound code

SoundEntry::play() calls StreamedSound::setFilterId(),
StreamSound::setFilterId() requires the underlying reference to be alive.
SoundQueue::handleTimer() checks that the stream is still alive
by calling SoundEntry::isFinished(). However, if the stream is finalized
just between calls to SoundEntry::isFinished() and SoundEntry::play(),
the sound mixer frees the stream leading to use-after-free in setFilterId().

Turn off the automatical disposing, delete the stream in SoundEntry::~SoundEntry().
This commit is contained in:
Evgeny Grechnikov 2018-10-16 00:49:07 +03:00
parent 45641f3de5
commit 8162309212
2 changed files with 11 additions and 5 deletions

View File

@ -446,8 +446,9 @@ LastExpress_ADPCMStream *SimpleSound::makeDecoder(Common::SeekableReadStream *in
return new LastExpress_ADPCMStream(in, DisposeAfterUse::YES, size, _blockSize, filterId);
}
void SimpleSound::play(Audio::AudioStream *as) {
g_system->getMixer()->playStream(Audio::Mixer::kPlainSoundType, &_handle, as);
void SimpleSound::play(Audio::AudioStream *as, DisposeAfterUse::Flag autofreeStream) {
g_system->getMixer()->playStream(Audio::Mixer::kPlainSoundType, &_handle, as,
-1, Audio::Mixer::kMaxChannelVolume, 0, autofreeStream);
}
//////////////////////////////////////////////////////////////////////////
@ -456,6 +457,7 @@ void SimpleSound::play(Audio::AudioStream *as) {
StreamedSound::StreamedSound() : _as(NULL), _loaded(false) {}
StreamedSound::~StreamedSound() {
delete _as;
_as = NULL;
}
@ -467,11 +469,15 @@ bool StreamedSound::load(Common::SeekableReadStream *stream, int32 filterId) {
loadHeader(stream);
if (_as) {
stop();
delete _as;
}
// Start decoding the input stream
_as = makeDecoder(stream, _size, filterId);
// Start playing the decoded audio stream
play(_as);
play(_as, DisposeAfterUse::NO);
_loaded = true;
@ -501,7 +507,7 @@ AppendableSound::AppendableSound() : SimpleSound() {
_finished = false;
// Start playing the decoded audio stream
play(_as);
play(_as, DisposeAfterUse::YES);
// Initialize the block size
// TODO: get it as an argument?

View File

@ -62,7 +62,7 @@ public:
protected:
void loadHeader(Common::SeekableReadStream *in);
LastExpress_ADPCMStream *makeDecoder(Common::SeekableReadStream *in, uint32 size, int32 filterId = -1) const;
void play(Audio::AudioStream *as);
void play(Audio::AudioStream *as, DisposeAfterUse::Flag autofreeStream);
uint32 _size; ///< data size
///< - NIS: size of all blocks, including those located in the matching LNK file