[Scummvm-git-logs] scummvm master -> 8162309212bbc287632fc375d1740a64733019fb

[grechnik]

This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
8162309212 LASTEXPRESS: fix race condition in sound code


Commit: 8162309212bbc287632fc375d1740a64733019fb
    https://github.com/scummvm/scummvm/commit/8162309212bbc287632fc375d1740a64733019fb
Author: Evgeny Grechnikov (diamondaz at yandex.ru)
Date: 2018-10-16T00:49:07+03:00

Commit Message:
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().

Changed paths:
    engines/lastexpress/data/snd.cpp
    engines/lastexpress/data/snd.h


diff --git a/engines/lastexpress/data/snd.cpp b/engines/lastexpress/data/snd.cpp
index e310baf..4cff837 100644
--- a/engines/lastexpress/data/snd.cpp
+++ b/engines/lastexpress/data/snd.cpp
@@ -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?
diff --git a/engines/lastexpress/data/snd.h b/engines/lastexpress/data/snd.h
index 19e5fda..23aae90 100644
--- a/engines/lastexpress/data/snd.h
+++ b/engines/lastexpress/data/snd.h
@@ -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