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

Commit 2650e966 authored by Jean-Michel Trivi's avatar Jean-Michel Trivi
Browse files

MediaPlayerService: prevent audio_attributes_t race conditions 

Access to audio attributes fields in Client and AudioOutput
 was not always locked.
Audio attributes field in AudioOutput cannot share the same pointer
 as Client because it can be indepently accessed. Save the
 attributes inside AudioOutput instead.

Bug 22672670

Change-Id: Ib1002b57b45cea44ff5e6eb115d581dc3beec006
parent 70c1f74f

media/libmediaplayerservice/MediaPlayerService.cpp

+21 −4
Original line number Diff line number Diff line
@@ -655,6 +655,7 @@ sp<MediaPlayerBase> MediaPlayerService::Client::setDataSource_pre( 
    }



    if (!p->hardwareOutput()) {

        Mutex::Autolock l(mLock);

        mAudioOutput = new AudioOutput(mAudioSessionId, IPCThreadState::self()->getCallingUid(),

                mPid, mAudioAttributes);

        static_cast<MediaPlayerInterface*>(p.get())->setAudioSink(mAudioOutput);
@@ -1101,6 +1102,9 @@ status_t MediaPlayerService::Client::setAudioAttributes_l(const Parcel &parcel) 
{

    if (mAudioAttributes != NULL) { free(mAudioAttributes); }

    mAudioAttributes = (audio_attributes_t *) calloc(1, sizeof(audio_attributes_t));

    if (mAudioAttributes == NULL) {

        return NO_MEMORY;

    }

    unmarshallAudioAttributes(parcel, mAudioAttributes);



    ALOGV("setAudioAttributes_l() usage=%d content=%d flags=0x%x tags=%s",
@@ -1337,7 +1341,6 @@ MediaPlayerService::AudioOutput::AudioOutput(int sessionId, int uid, int pid, 
      mCallbackData(NULL),

      mBytesWritten(0),

      mStreamType(AUDIO_STREAM_MUSIC),

      mAttributes(attr),

      mLeftVolume(1.0),

      mRightVolume(1.0),

      mPlaybackRate(AUDIO_PLAYBACK_RATE_DEFAULT),
@@ -1353,8 +1356,14 @@ MediaPlayerService::AudioOutput::AudioOutput(int sessionId, int uid, int pid, 
{

    ALOGV("AudioOutput(%d)", sessionId);

    if (attr != NULL) {

        mAttributes = (audio_attributes_t *) calloc(1, sizeof(audio_attributes_t));

        if (mAttributes != NULL) {

            memcpy(mAttributes, attr, sizeof(audio_attributes_t));

            mStreamType = audio_attributes_to_stream_type(attr);

        }

    } else {

        mAttributes = NULL;

    }



    setMinBufferCount();

}
@@ -1362,6 +1371,7 @@ MediaPlayerService::AudioOutput::AudioOutput(int sessionId, int uid, int pid, 
MediaPlayerService::AudioOutput::~AudioOutput()

{

    close();

    free(mAttributes);

    delete mCallbackData;

}
@@ -1468,14 +1478,21 @@ String8 MediaPlayerService::AudioOutput::getParameters(const String8& keys) 


void MediaPlayerService::AudioOutput::setAudioAttributes(const audio_attributes_t * attributes) {

    Mutex::Autolock lock(mLock);

    mAttributes = attributes;

    if (attributes != NULL) {

    if (attributes == NULL) {

        free(mAttributes);

        mAttributes = NULL;

    } else {

        if (mAttributes == NULL) {

            mAttributes = (audio_attributes_t *) calloc(1, sizeof(audio_attributes_t));

        }

        memcpy(mAttributes, attributes, sizeof(audio_attributes_t));

        mStreamType = audio_attributes_to_stream_type(attributes);

    }

}



void MediaPlayerService::AudioOutput::setAudioStreamType(audio_stream_type_t streamType)

{

    Mutex::Autolock lock(mLock);

    // do not allow direct stream type modification if attributes have been set

    if (mAttributes == NULL) {

        mStreamType = streamType;

media/libmediaplayerservice/MediaPlayerService.h

+1 −1
Original line number Diff line number Diff line
@@ -142,7 +142,7 @@ class MediaPlayerService : public BnMediaPlayerService 
        CallbackData *          mCallbackData;

        uint64_t                mBytesWritten;

        audio_stream_type_t     mStreamType;

        const audio_attributes_t *mAttributes;

        audio_attributes_t *    mAttributes;

        float                   mLeftVolume;

        float                   mRightVolume;

        AudioPlaybackRate       mPlaybackRate;