From 2650e9661ea1608dfb6b58bc640a66cdbbb6ae58 Mon Sep 17 00:00:00 2001
From: Jean-Michel Trivi <jmtrivi@google.com>
Date: Wed, 22 Jul 2015 18:14:02 -0700
Subject: 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 | 25 ++++++++++++++++++----
 media/libmediaplayerservice/MediaPlayerService.h   |  2 +-
 2 files changed, 22 insertions(+), 5 deletions(-)

(limited to 'media/libmediaplayerservice')

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;
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 @@ 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;
-- 
cgit v1.1