From b9481d8cf6f3ade96ed062dc3f601c777fe4430f Mon Sep 17 00:00:00 2001
From: Eric Laurent <elaurent@google.com>
Date: Thu, 17 Sep 2009 05:12:56 -0700
Subject: [PATCH] Fix issue 2127371: Possible race condition in
 AudioFlinger::openRecord() when a Track is being destroyed.

The fix consists in locking AudioFlinger::mLock mutex in the TrackBase destructor before clearing the strong pointer to the shared memory client. The mutex is not locked in removeclient() any more which implies that we must make sure that the Client destructor is always called from the TrackBase destructor or that we hold the mLock mutex before calling deleting the Client.
---
 libs/audioflinger/AudioFlinger.cpp | 20 +++++++++++++++-----
 libs/audioflinger/AudioFlinger.h   |  4 +++-
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/libs/audioflinger/AudioFlinger.cpp b/libs/audioflinger/AudioFlinger.cpp
index add358b0ca396..e534447abfdaf 100644
--- a/libs/audioflinger/AudioFlinger.cpp
+++ b/libs/audioflinger/AudioFlinger.cpp
@@ -307,6 +307,9 @@ sp<IAudioTrack> AudioFlinger::createTrack(
     if (lStatus == NO_ERROR) {
         trackHandle = new TrackHandle(track);
     } else {
+        // remove local strong reference to Client before deleting the Track so that the Client
+        // destructor is called by the TrackBase destructor with mLock held
+        client.clear();
         track.clear();
     }
 
@@ -707,10 +710,10 @@ void AudioFlinger::audioConfigChanged_l(int event, const sp<ThreadBase>& thread,
     }
 }
 
-void AudioFlinger::removeClient(pid_t pid)
+// removeClient_l() must be called with AudioFlinger::mLock held
+void AudioFlinger::removeClient_l(pid_t pid)
 {
-    LOGV("removeClient() pid %d, tid %d, calling tid %d", pid, gettid(), IPCThreadState::self()->getCallingPid());
-    Mutex::Autolock _l(mLock);
+    LOGV("removeClient_l() pid %d, tid %d, calling tid %d", pid, gettid(), IPCThreadState::self()->getCallingPid());
     mClients.removeItem(pid);
 }
 
@@ -2078,7 +2081,10 @@ AudioFlinger::ThreadBase::TrackBase::~TrackBase()
         }
     }
     mCblkMemory.clear();            // and free the shared memory
-    mClient.clear();
+    if (mClient != NULL) {
+        Mutex::Autolock _l(mClient->audioFlinger()->mLock);
+        mClient.clear();
+    }
 }
 
 void AudioFlinger::ThreadBase::TrackBase::releaseBuffer(AudioBufferProvider::Buffer* buffer)
@@ -2712,9 +2718,10 @@ AudioFlinger::Client::Client(const sp<AudioFlinger>& audioFlinger, pid_t pid)
     // 1 MB of address space is good for 32 tracks, 8 buffers each, 4 KB/buffer
 }
 
+// Client destructor must be called with AudioFlinger::mLock held
 AudioFlinger::Client::~Client()
 {
-    mAudioFlinger->removeClient(mPid);
+    mAudioFlinger->removeClient_l(mPid);
 }
 
 const sp<MemoryDealer>& AudioFlinger::Client::heap() const
@@ -2820,6 +2827,9 @@ sp<IAudioRecord> AudioFlinger::openRecord(
                                                    format, channelCount, frameCount, flags);
     }
     if (recordTrack->getCblk() == NULL) {
+        // remove local strong reference to Client before deleting the RecordTrack so that the Client
+        // destructor is called by the TrackBase destructor with mLock held
+        client.clear();
         recordTrack.clear();
         lStatus = NO_MEMORY;
         goto Exit;
diff --git a/libs/audioflinger/AudioFlinger.h b/libs/audioflinger/AudioFlinger.h
index 7a6641f9f78df..369901950d3ae 100644
--- a/libs/audioflinger/AudioFlinger.h
+++ b/libs/audioflinger/AudioFlinger.h
@@ -189,6 +189,8 @@ private:
         virtual             ~Client();
         const sp<MemoryDealer>&     heap() const;
         pid_t               pid() const { return mPid; }
+        sp<AudioFlinger>    audioFlinger() { return mAudioFlinger; }
+
     private:
                             Client(const Client&);
                             Client& operator = (const Client&);
@@ -641,7 +643,7 @@ private:
     friend class PlaybackThread::Track;
 
 
-                void        removeClient(pid_t pid);
+                void        removeClient_l(pid_t pid);
 
 
     // record thread