Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0f8ab670 authored by Eric Laurent's avatar Eric Laurent
Browse files

Fix issue 2127371: Possible race condition in AudioFlinger::openRecord() when... 

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.
parent 5140a13b

libs/audioflinger/AudioFlinger.cpp

+15 −5
Original line number Diff line number Diff line
@@ -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,8 +2081,11 @@ AudioFlinger::ThreadBase::TrackBase::~TrackBase() 
        }

    }

    mCblkMemory.clear();            // and free the shared memory

    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;

libs/audioflinger/AudioFlinger.h

+3 −1
Original line number Diff line number Diff line
@@ -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