commit 2650e9661ea1608dfb6b58bc640a66cdbbb6ae58
author Jean-Michel Trivi <jmtrivi@google.com> Wed Jul 22 18:14:02 2015 -0700
committer Jean-Michel Trivi <jmtrivi@google.com> Thu Jul 23 17:44:57 2015 -0700
tree 70b66e87a1061e0b842ca054ca3482950c69ed0e
parent 70c1f74f3d4c43b67949f3e742195d60f4677462

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

media/libmediaplayerservice/MediaPlayerService.cpp

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 @@
     }
 
     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 @@
 {
     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 @@
       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 @@
 {
     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()
 {
     close();
+    free(mAttributes);
     delete mCallbackData;
 }
 
@@ -1468,14 +1478,21 @@
 
 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

diff --git a/media/libmediaplayerservice/MediaPlayerService.h b/media/libmediaplayerservice/MediaPlayerService.h
index 1c32597..60d4617 100644
--- a/media/libmediaplayerservice/MediaPlayerService.h
+++ b/media/libmediaplayerservice/MediaPlayerService.h
@@ -142,7 +142,7 @@
         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;