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
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 @@
// 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 @@
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 @@
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 @@
if (halFrames == 0 || dspFrames == 0) {
return BAD_VALUE;
}
+ Mutex::Autolock _l(mLock);
if (initCheck() != NO_ERROR) {
return INVALID_OPERATION;
}
@@ -1547,6 +1554,29 @@
}
+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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
{
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 @@
(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 @@
(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 @@
(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 @@
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 @@
~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 @@
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; }