From 8162309212bbc287632fc375d1740a64733019fb Mon Sep 17 00:00:00 2001
From: Evgeny Grechnikov
Date: Tue, 16 Oct 2018 00:49:07 +0300
Subject: 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().
---
 engines/lastexpress/data/snd.cpp | 14 ++++++++++----
 engines/lastexpress/data/snd.h   |  2 +-
 2 files changed, 11 insertions(+), 5 deletions(-)

(limited to 'engines/lastexpress')

diff --git a/engines/lastexpress/data/snd.cpp b/engines/lastexpress/data/snd.cpp
index e310bafd18..4cff837193 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 19e5fda9c9..23aae905ac 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
-- 
cgit v1.2.3