Prevent MediaPlayerService::Client's use-after-free am: 028cf0973f am: 45ff66df1b

am: ede9ead9e7

Change-Id: Idcb724463c4cd8bf4afff78044986803d3417175

media/libmediaplayerservice/MediaPlayerFactory.cpp

@@ -126,7 +126,7 @@ player_type MediaPlayerFactory::getPlayerType(const sp<IMediaPlayer>& client,
 
 sp<MediaPlayerBase> MediaPlayerFactory::createPlayer(
         player_type playerType,
-        void* cookie,
+        const wp<IMediaPlayer> &client,
         notify_callback_f notifyFunc,
         pid_t pid) {
     sp<MediaPlayerBase> p;
@@ -152,7 +152,7 @@ sp<MediaPlayerBase> MediaPlayerFactory::createPlayer(
 
     init_result = p->initCheck();
     if (init_result == NO_ERROR) {
-        p->setNotifyCallback(cookie, notifyFunc);
+        p->setNotifyCallback(client, notifyFunc);
     } else {
         ALOGE("Failed to create player object of type %d, initCheck failed"
               " (res = %d)", playerType, init_result);

media/libmediaplayerservice/MediaPlayerFactory.h

@@ -65,7 +65,7 @@ class MediaPlayerFactory {
                                      const sp<DataSource> &source);
 
     static sp<MediaPlayerBase> createPlayer(player_type playerType,
-                                            void* cookie,
+                                            const wp<IMediaPlayer> &client,
                                             notify_callback_f notifyFunc,
                                             pid_t pid);
 

media/libmediaplayerservice/MediaPlayerService.cpp

@@ -1430,12 +1430,13 @@ status_t MediaPlayerService::Client::getRetransmitEndpoint(
 }
 
 void MediaPlayerService::Client::notify(
-        void* cookie, int msg, int ext1, int ext2, const Parcel *obj)
+        const wp<IMediaPlayer> &listener, int msg, int ext1, int ext2, const Parcel *obj)
 {
-    Client* client = static_cast<Client*>(cookie);
-    if (client == NULL) {
+    sp<IMediaPlayer> spListener = listener.promote();
+    if (spListener == NULL) {
         return;
     }
+    Client* client = static_cast<Client*>(spListener.get());
 
     sp<IMediaPlayerClient> c;
     sp<Client> nextClient;
@@ -1483,7 +1484,7 @@ void MediaPlayerService::Client::notify(
     }
 
     if (c != NULL) {
-        ALOGV("[%d] notify (%p, %d, %d, %d)", client->mConnId, cookie, msg, ext1, ext2);
+        ALOGV("[%d] notify (%p, %d, %d, %d)", client->mConnId, spListener.get(), msg, ext1, ext2);
         c->notify(msg, ext1, ext2, obj);
     }
 }
@@ -1541,7 +1542,7 @@ status_t MediaPlayerService::Client::releaseDrm()
 #if CALLBACK_ANTAGONIZER
 const int Antagonizer::interval = 10000; // 10 msecs
 
-Antagonizer::Antagonizer(notify_callback_f cb, void* client) :
+Antagonizer::Antagonizer(notify_callback_f cb, const wp<IMediaPlayer> &client) :
     mExit(false), mActive(false), mClient(client), mCb(cb)
 {
     createThread(callbackThread, this);

media/libmediaplayerservice/MediaPlayerService.h

@@ -51,7 +51,7 @@ class MediaRecorderClient;
 #if CALLBACK_ANTAGONIZER
 class Antagonizer {
 public:
-    Antagonizer(notify_callback_f cb, void* client);
+    Antagonizer(notify_callback_f cb, const wp<IMediaPlayer> &client);
     void start() { mActive = true; }
     void stop() { mActive = false; }
     void kill();
@@ -63,7 +63,7 @@ private:
     Condition           mCondition;
     bool                mExit;
     bool                mActive;
-    void*               mClient;
+    wp<IMediaPlayer>    mClient;
     notify_callback_f   mCb;
 };
 #endif
@@ -365,7 +365,7 @@ private:
         status_t                setDataSource_post(const sp<MediaPlayerBase>& p,
                                                    status_t status);
 
-        static  void            notify(void* cookie, int msg,
+        static  void            notify(const wp<IMediaPlayer> &cookie, int msg,
                                        int ext1, int ext2, const Parcel *obj);
 
                 pid_t           pid() const { return mPid; }

media/libmediaplayerservice/include/MediaPlayerInterface.h

@@ -67,7 +67,7 @@ enum player_type {
 #define AUDIO_SINK_MIN_DEEP_BUFFER_DURATION_US 5000000
 
 // callback mechanism for passing messages to MediaPlayer object
-typedef void (*notify_callback_f)(void* cookie,
+typedef void (*notify_callback_f)(const wp<IMediaPlayer> &listener,
         int msg, int ext1, int ext2, const Parcel *obj);
 
 // abstract base class - use MediaPlayerInterface
@@ -152,7 +152,7 @@ public:
         virtual sp<VolumeShaper::State> getVolumeShaperState(int id);
     };
 
-                        MediaPlayerBase() : mCookie(0), mNotify(0) {}
+                        MediaPlayerBase() : mClient(0), mNotify(0) {}
     virtual             ~MediaPlayerBase() {}
     virtual status_t    initCheck() = 0;
     virtual bool        hardwareOutput() = 0;
@@ -263,22 +263,22 @@ public:
     };
 
     void        setNotifyCallback(
-            void* cookie, notify_callback_f notifyFunc) {
+            const wp<IMediaPlayer> &client, notify_callback_f notifyFunc) {
         Mutex::Autolock autoLock(mNotifyLock);
-        mCookie = cookie; mNotify = notifyFunc;
+        mClient = client; mNotify = notifyFunc;
     }
 
     void        sendEvent(int msg, int ext1=0, int ext2=0,
                           const Parcel *obj=NULL) {
         notify_callback_f notifyCB;
-        void* cookie;
+        wp<IMediaPlayer> client;
         {
             Mutex::Autolock autoLock(mNotifyLock);
             notifyCB = mNotify;
-            cookie = mCookie;
+            client = mClient;
         }
 
-        if (notifyCB) notifyCB(cookie, msg, ext1, ext2, obj);
+        if (notifyCB) notifyCB(client, msg, ext1, ext2, obj);
     }
 
     virtual status_t dump(int /* fd */, const Vector<String16>& /* args */) const {
@@ -297,7 +297,7 @@ private:
     friend class MediaPlayerService;
 
     Mutex               mNotifyLock;
-    void*               mCookie;
+    wp<IMediaPlayer>    mClient;
     notify_callback_f   mNotify;
 };