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

1 files changed, 21 insertions, 4 deletions

diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp
index d49ba93..56521a2 100644
--- a/media/libmediaplayerservice/MediaPlayerService.cpp
+++ b/media/libmediaplayerservice/MediaPlayerService.cpp
@@ -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,7 +1356,13 @@ MediaPlayerService::AudioOutput::AudioOutput(int sessionId, int uid, int pid,
 {
     ALOGV("AudioOutput(%d)", sessionId);
     if (attr != NULL) {
-        mStreamType = audio_attributes_to_stream_type(attr);
+        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;