Prevent MediaPlayerService::Client's use-after-free
Test: make cts -j123 && cts-tradefed run cts-dev -m \
CtsMediaTestCases --compatibility:module-arg \
CtsMediaTestCases:include-annotation:\
android.platform.test.annotations.RequiresDevice
Bug: 70546581
Merged-In: Ia142a7735c6685eb67b2c00917c0ed5ea7e0da9e
Change-Id: Ia142a7735c6685eb67b2c00917c0ed5ea7e0da9e
(cherry picked from commit d27ad6bae3215db0ed174f5f036339c9515fbba7)
diff --git a/media/libmediaplayerservice/MediaPlayerFactory.cpp b/media/libmediaplayerservice/MediaPlayerFactory.cpp
index 0a9f791..88064d9 100644
--- a/media/libmediaplayerservice/MediaPlayerFactory.cpp
+++ b/media/libmediaplayerservice/MediaPlayerFactory.cpp
@@ -126,7 +126,7 @@
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 @@
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);
diff --git a/media/libmediaplayerservice/MediaPlayerFactory.h b/media/libmediaplayerservice/MediaPlayerFactory.h
index e22a56f..6aedeb2 100644
--- a/media/libmediaplayerservice/MediaPlayerFactory.h
+++ b/media/libmediaplayerservice/MediaPlayerFactory.h
@@ -65,7 +65,7 @@
const sp<DataSource> &source);
static sp<MediaPlayerBase> createPlayer(player_type playerType,
- void* cookie,
+ const wp<IMediaPlayer> &client,
notify_callback_f notifyFunc,
pid_t pid);
diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp
index 11f29cb..a963424 100644
--- a/media/libmediaplayerservice/MediaPlayerService.cpp
+++ b/media/libmediaplayerservice/MediaPlayerService.cpp
@@ -1430,12 +1430,13 @@
}
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 @@
}
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 @@
#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);
diff --git a/media/libmediaplayerservice/MediaPlayerService.h b/media/libmediaplayerservice/MediaPlayerService.h
index e35d098..f043f32 100644
--- a/media/libmediaplayerservice/MediaPlayerService.h
+++ b/media/libmediaplayerservice/MediaPlayerService.h
@@ -51,7 +51,7 @@
#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 @@
Condition mCondition;
bool mExit;
bool mActive;
- void* mClient;
+ wp<IMediaPlayer> mClient;
notify_callback_f mCb;
};
#endif
@@ -365,7 +365,7 @@
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; }
diff --git a/media/libmediaplayerservice/include/MediaPlayerInterface.h b/media/libmediaplayerservice/include/MediaPlayerInterface.h
index a01f7f2..c21ef42 100644
--- a/media/libmediaplayerservice/include/MediaPlayerInterface.h
+++ b/media/libmediaplayerservice/include/MediaPlayerInterface.h
@@ -67,7 +67,7 @@
#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 @@
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 @@
};
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 @@
friend class MediaPlayerService;
Mutex mNotifyLock;
- void* mCookie;
+ wp<IMediaPlayer> mClient;
notify_callback_f mNotify;
};