AudioFlinger: protect input/output stream access

Some methods would not check that the output orinput stream of a thread
was still valid before calling functions on its interface.
This could cause a crash if those methods where called while the output or
input was being closed by another thread.

Make sure that the output or input stream pointer is cleared before closing the
stream.
Always check that the output or input pointer is not null before calling
functions at the stream interface.
Generalize the use of initCheck() method to verify that the output or input
stream is not null.

Change-Id: I9d9ca6b744d011bcf3a7bbacb4a581ac1477bfa5

2 files changed, 106 insertions, 21 deletions

diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index ec45530..b11ec63 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -1230,12 +1230,13 @@ status_t AudioFlinger::PlaybackThread::dumpInternals(int fd, const Vector<String
 // Thread virtuals
 status_t AudioFlinger::PlaybackThread::readyToRun()
 {
-    if (mSampleRate == 0) {
+    status_t status = initCheck();
+    if (status == NO_ERROR) {
+        LOGI("AudioFlinger's thread %p ready to run", this);
+    } else {
         LOGE("No working audio driver found.");
-        return NO_INIT;
     }
-    LOGI("AudioFlinger's thread %p ready to run", this);
-    return NO_ERROR;
+    return status;
 }
 
 void AudioFlinger::PlaybackThread::onFirstRef()
@@ -1329,10 +1330,10 @@ Exit:
 
 uint32_t AudioFlinger::PlaybackThread::latency() const
 {
-    if (mOutput) {
+    Mutex::Autolock _l(mLock);
+    if (initCheck() == NO_ERROR) {
         return mOutput->stream->get_latency(mOutput->stream);
-    }
-    else {
+    } else {
         return 0;
     }
 }
@@ -1433,16 +1434,21 @@ void AudioFlinger::PlaybackThread::removeTrack_l(const sp<Track>& track)
 
 String8 AudioFlinger::PlaybackThread::getParameters(const String8& keys)
 {
-    String8 out_s8;
+    String8 out_s8 = String8("");
     char *s;
 
+    Mutex::Autolock _l(mLock);
+    if (initCheck() != NO_ERROR) {
+        return out_s8;
+    }
+
     s = mOutput->stream->common.get_parameters(&mOutput->stream->common, keys.string());
     out_s8 = String8(s);
     free(s);
     return out_s8;
 }
 
-// destroyTrack_l() must be called with AudioFlinger::mLock held
+// audioConfigChanged_l() must be called with AudioFlinger::mLock held
 void AudioFlinger::PlaybackThread::audioConfigChanged_l(int event, int param) {
     AudioSystem::OutputDescriptor desc;
     void *param2 = 0;
@@ -1501,6 +1507,7 @@ status_t AudioFlinger::PlaybackThread::getRenderPosition(uint32_t *halFrames, ui
     if (halFrames == 0 || dspFrames == 0) {
         return BAD_VALUE;
     }
+    Mutex::Autolock _l(mLock);
     if (initCheck() != NO_ERROR) {
         return INVALID_OPERATION;
     }
@@ -1547,6 +1554,29 @@ uint32_t AudioFlinger::PlaybackThread::getStrategyForSession_l(int sessionId)
 }
 
 
+AudioFlinger::AudioStreamOut* AudioFlinger::PlaybackThread::getOutput()
+{
+    Mutex::Autolock _l(mLock);
+    return mOutput;
+}
+
+AudioFlinger::AudioStreamOut* AudioFlinger::PlaybackThread::clearOutput()
+{
+    Mutex::Autolock _l(mLock);
+    AudioStreamOut *output = mOutput;
+    mOutput = NULL;
+    return output;
+}
+
+// this method must always be called either with ThreadBase mLock held or inside the thread loop
+audio_stream_t* AudioFlinger::PlaybackThread::stream()
+{
+    if (mOutput == NULL) {
+        return NULL;
+    }
+    return &mOutput->stream->common;
+}
+
 // ----------------------------------------------------------------------------
 
 AudioFlinger::MixerThread::MixerThread(const sp<AudioFlinger>& audioFlinger, AudioStreamOut* output, int id, uint32_t device)
@@ -3921,6 +3951,13 @@ void AudioFlinger::RecordThread::onFirstRef()
     run(mName, PRIORITY_URGENT_AUDIO);
 }
 
+status_t AudioFlinger::RecordThread::readyToRun()
+{
+    status_t status = initCheck();
+    LOGW_IF(status != NO_ERROR,"RecordThread %p could not initialize", this);
+    return status;
+}
+
 bool AudioFlinger::RecordThread::threadLoop()
 {
     AudioBufferProvider::Buffer buffer;
@@ -4400,7 +4437,12 @@ bool AudioFlinger::RecordThread::checkForNewParameters_l()
 String8 AudioFlinger::RecordThread::getParameters(const String8& keys)
 {
     char *s;
-    String8 out_s8;
+    String8 out_s8 = String8();
+
+    Mutex::Autolock _l(mLock);
+    if (initCheck() != NO_ERROR) {
+        return out_s8;
+    }
 
     s = mInput->stream->common.get_parameters(&mInput->stream->common, keys.string());
     out_s8 = String8(s);
@@ -4472,6 +4514,11 @@ void AudioFlinger::RecordThread::readInputParameters()
 
 unsigned int AudioFlinger::RecordThread::getInputFramesLost()
 {
+    Mutex::Autolock _l(mLock);
+    if (initCheck() != NO_ERROR) {
+        return 0;
+    }
+
     return mInput->stream->get_input_frames_lost(mInput->stream);
 }
 
@@ -4490,6 +4537,30 @@ uint32_t AudioFlinger::RecordThread::hasAudioSession(int sessionId)
     return result;
 }
 
+AudioFlinger::AudioStreamIn* AudioFlinger::RecordThread::getInput()
+{
+    Mutex::Autolock _l(mLock);
+    return mInput;
+}
+
+AudioFlinger::AudioStreamIn* AudioFlinger::RecordThread::clearInput()
+{
+    Mutex::Autolock _l(mLock);
+    AudioStreamIn *input = mInput;
+    mInput = NULL;
+    return input;
+}
+
+// this method must always be called either with ThreadBase mLock held or inside the thread loop
+audio_stream_t* AudioFlinger::RecordThread::stream()
+{
+    if (mInput == NULL) {
+        return NULL;
+    }
+    return &mInput->stream->common;
+}
+
+
 // ----------------------------------------------------------------------------
 
 int AudioFlinger::openOutput(uint32_t *pDevices,
@@ -4613,7 +4684,8 @@ status_t AudioFlinger::closeOutput(int output)
     thread->exit();
 
     if (thread->type() != ThreadBase::DUPLICATING) {
-        AudioStreamOut *out = thread->getOutput();
+        AudioStreamOut *out = thread->clearOutput();
+        // from now on thread->mOutput is NULL
         out->hwDev->close_output_stream(out->hwDev, out->stream);
         delete out;
     }
@@ -4753,7 +4825,8 @@ status_t AudioFlinger::closeInput(int input)
     }
     thread->exit();
 
-    AudioStreamIn *in = thread->getInput();
+    AudioStreamIn *in = thread->clearInput();
+    // from now on thread->mInput is NULL
     in->hwDev->close_input_stream(in->hwDev, in->stream);
     delete in;
 
@@ -4831,7 +4904,8 @@ AudioFlinger::PlaybackThread *AudioFlinger::primaryPlaybackThread_l()
 {
     for (size_t i = 0; i < mPlaybackThreads.size(); i++) {
         PlaybackThread *thread = mPlaybackThreads.valueAt(i).get();
-        if (thread->getOutput()->hwDev == mPrimaryHardwareDev) {
+        AudioStreamOut *output = thread->getOutput();
+        if (output != NULL && output->hwDev == mPrimaryHardwareDev) {
             return thread;
         }
     }
@@ -5609,7 +5683,10 @@ AudioFlinger::EffectModule::~EffectModule()
                 (mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_POST_PROC) {
             sp<ThreadBase> thread = mThread.promote();
             if (thread != 0) {
-                thread->stream()->remove_audio_effect(thread->stream(), mEffectInterface);
+                audio_stream_t *stream = thread->stream();
+                if (stream != NULL) {
+                    stream->remove_audio_effect(stream, mEffectInterface);
+                }
             }
         }
         // release effect engine
@@ -5900,7 +5977,10 @@ status_t AudioFlinger::EffectModule::start_l()
              (mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_POST_PROC)) {
         sp<ThreadBase> thread = mThread.promote();
         if (thread != 0) {
-            thread->stream()->add_audio_effect(thread->stream(), mEffectInterface);
+            audio_stream_t *stream = thread->stream();
+            if (stream != NULL) {
+                stream->add_audio_effect(stream, mEffectInterface);
+            }
         }
     }
     return status;
@@ -5933,7 +6013,10 @@ status_t AudioFlinger::EffectModule::stop_l()
              (mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_POST_PROC)) {
         sp<ThreadBase> thread = mThread.promote();
         if (thread != 0) {
-            thread->stream()->remove_audio_effect(thread->stream(), mEffectInterface);
+            audio_stream_t *stream = thread->stream();
+            if (stream != NULL) {
+                stream->remove_audio_effect(stream, mEffectInterface);
+            }
         }
     }
     return status;
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 7b6215f..80a9945 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -688,8 +688,9 @@ private:
                                     int sessionId,
                                     status_t *status);
 
-                    AudioStreamOut* getOutput() { return mOutput; }
-                    virtual audio_stream_t* stream() { return &mOutput->stream->common; }
+                    AudioStreamOut* getOutput();
+                    AudioStreamOut* clearOutput();
+                    virtual audio_stream_t* stream();
 
                     void        suspend() { mSuspended++; }
                     void        restore() { if (mSuspended) mSuspended--; }
@@ -930,7 +931,7 @@ private:
                 ~RecordThread();
 
         virtual bool        threadLoop();
-        virtual status_t    readyToRun() { return NO_ERROR; }
+        virtual status_t    readyToRun();
         virtual void        onFirstRef();
 
         virtual status_t    initCheck() const { return (mInput == 0) ? NO_INIT : NO_ERROR; }
@@ -947,8 +948,9 @@ private:
                 status_t    start(RecordTrack* recordTrack);
                 void        stop(RecordTrack* recordTrack);
                 status_t    dump(int fd, const Vector<String16>& args);
-                AudioStreamIn* getInput() { return mInput; }
-                virtual audio_stream_t* stream() { return &mInput->stream->common; }
+                AudioStreamIn* getInput();
+                AudioStreamIn* clearInput();
+                virtual audio_stream_t* stream();
 
 
                 void        setTrack(RecordTrack *recordTrack) { mTrack = recordTrack; }