Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c4588165 authored by Pawin Vongmasa's avatar Pawin Vongmasa
Browse files

RESTRICT AUTOMERGE 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: Ia3a8eb99c2faf6935c63800ba08f65970cede48e
Change-Id: Ia142a7735c6685eb67b2c00917c0ed5ea7e0da9e
parent fb41e97c
Loading
Loading
Loading
Loading
+17 −15
Original line number Diff line number Diff line
@@ -64,14 +64,17 @@ enum player_type {
// duration below which we do not allow deep audio buffering
#define AUDIO_SINK_MIN_DEEP_BUFFER_DURATION_US 5000000

// callback mechanism for passing messages to MediaPlayer object
typedef void (*notify_callback_f)(void* cookie,
        int msg, int ext1, int ext2, const Parcel *obj);

// abstract base class - use MediaPlayerInterface
class MediaPlayerBase : public RefBase
{
public:
    // callback mechanism for passing messages to MediaPlayer object
    class Listener : public RefBase {
    public:
        virtual void notify(int msg, int ext1, int ext2, const Parcel *obj) = 0;
        virtual ~Listener() {}
    };

    // AudioSink: abstraction layer for audio output
    class AudioSink : public RefBase {
    public:
@@ -129,7 +132,7 @@ public:
        virtual String8     getParameters(const String8& keys) { return String8::empty(); };
    };

                        MediaPlayerBase() : mCookie(0), mNotify(0) {}
                        MediaPlayerBase() {}
    virtual             ~MediaPlayerBase() {}
    virtual status_t    initCheck() = 0;
    virtual bool        hardwareOutput() = 0;
@@ -201,22 +204,22 @@ public:
    };

    void        setNotifyCallback(
            void* cookie, notify_callback_f notifyFunc) {
            const sp<Listener> &listener) {
        Mutex::Autolock autoLock(mNotifyLock);
        mCookie = cookie; mNotify = notifyFunc;
        mListener = listener;
    }

    void        sendEvent(int msg, int ext1=0, int ext2=0,
                          const Parcel *obj=NULL) {
        notify_callback_f notifyCB;
        void* cookie;
        sp<Listener> listener;
        {
            Mutex::Autolock autoLock(mNotifyLock);
            notifyCB = mNotify;
            cookie = mCookie;
            listener = mListener;
        }

        if (notifyCB) notifyCB(cookie, msg, ext1, ext2, obj);
        if (listener != NULL) {
            listener->notify(msg, ext1, ext2, obj);
        }
    }

    virtual status_t dump(int fd, const Vector<String16> &args) const {
@@ -227,8 +230,7 @@ private:
    friend class MediaPlayerService;

    Mutex        mNotifyLock;
    void*               mCookie;
    notify_callback_f   mNotify;
    sp<Listener> mListener;
};

// Implement this class for media players that use the AudioFlinger software mixer
+2 −3
Original line number Diff line number Diff line
@@ -135,8 +135,7 @@ player_type MediaPlayerFactory::getPlayerType(const sp<IMediaPlayer>& client,

sp<MediaPlayerBase> MediaPlayerFactory::createPlayer(
        player_type playerType,
        void* cookie,
        notify_callback_f notifyFunc) {
        const sp<MediaPlayerBase::Listener> &listener) {
    sp<MediaPlayerBase> p;
    IFactory* factory;
    status_t init_result;
@@ -160,7 +159,7 @@ sp<MediaPlayerBase> MediaPlayerFactory::createPlayer(

    init_result = p->initCheck();
    if (init_result == NO_ERROR) {
        p->setNotifyCallback(cookie, notifyFunc);
        p->setNotifyCallback(listener);
    } else {
        ALOGE("Failed to create player object of type %d, initCheck failed"
              " (res = %d)", playerType, init_result);
+1 −2
Original line number Diff line number Diff line
@@ -59,8 +59,7 @@ class MediaPlayerFactory {
                                     const sp<IStreamSource> &source);

    static sp<MediaPlayerBase> createPlayer(player_type playerType,
                                            void* cookie,
                                            notify_callback_f notifyFunc);
                                            const sp<MediaPlayerBase::Listener> &listener);

    static void registerBuiltinFactories();

+28 −32
Original line number Diff line number Diff line
@@ -617,10 +617,11 @@ MediaPlayerService::Client::Client(
    mUID = uid;
    mRetransmitEndpointValid = false;
    mAudioAttributes = NULL;
    mListener = new Listener(this);

#if CALLBACK_ANTAGONIZER
    ALOGD("create Antagonizer");
    mAntagonizer = new Antagonizer(notify, this);
    mAntagonizer = new Antagonizer(mListener);
#endif
}

@@ -656,7 +657,7 @@ void MediaPlayerService::Client::disconnect()
    // and reset the player. We assume the player will serialize
    // access to itself if necessary.
    if (p != 0) {
        p->setNotifyCallback(0, 0);
        p->setNotifyCallback(0);
#if CALLBACK_ANTAGONIZER
        ALOGD("kill Antagonizer");
        mAntagonizer->kill();
@@ -678,7 +679,7 @@ sp<MediaPlayerBase> MediaPlayerService::Client::createPlayer(player_type playerT
        p.clear();
    }
    if (p == NULL) {
        p = MediaPlayerFactory::createPlayer(playerType, this, notify);
        p = MediaPlayerFactory::createPlayer(playerType, mListener);
    }

    if (p != NULL) {
@@ -1221,22 +1222,17 @@ status_t MediaPlayerService::Client::getRetransmitEndpoint(
}

void MediaPlayerService::Client::notify(
        void* cookie, int msg, int ext1, int ext2, const Parcel *obj)
        int msg, int ext1, int ext2, const Parcel *obj)
{
    Client* client = static_cast<Client*>(cookie);
    if (client == NULL) {
        return;
    }

    sp<IMediaPlayerClient> c;
    {
        Mutex::Autolock l(client->mLock);
        c = client->mClient;
        if (msg == MEDIA_PLAYBACK_COMPLETE && client->mNextClient != NULL) {
            if (client->mAudioOutput != NULL)
                client->mAudioOutput->switchToNextOutput();
            client->mNextClient->start();
            client->mNextClient->mClient->notify(MEDIA_INFO, MEDIA_INFO_STARTED_AS_NEXT, 0, obj);
        Mutex::Autolock l(mLock);
        c = mClient;
        if (msg == MEDIA_PLAYBACK_COMPLETE && mNextClient != NULL) {
            if (mAudioOutput != NULL)
                mAudioOutput->switchToNextOutput();
            mNextClient->start();
            mNextClient->mClient->notify(MEDIA_INFO, MEDIA_INFO_STARTED_AS_NEXT, 0, obj);
        }
    }

@@ -1244,17 +1240,17 @@ void MediaPlayerService::Client::notify(
        MEDIA_INFO_METADATA_UPDATE == ext1) {
        const media::Metadata::Type metadata_type = ext2;

        if(client->shouldDropMetadata(metadata_type)) {
        if(shouldDropMetadata(metadata_type)) {
            return;
        }

        // Update the list of metadata that have changed. getMetadata
        // also access mMetadataUpdated and clears it.
        client->addNewMetadataUpdate(metadata_type);
        addNewMetadataUpdate(metadata_type);
    }

    if (c != NULL) {
        ALOGV("[%d] notify (%p, %d, %d, %d)", client->mConnId, cookie, msg, ext1, ext2);
        ALOGV("[%d] notify (%d, %d, %d)", mConnId, msg, ext1, ext2);
        c->notify(msg, ext1, ext2, obj);
    }
}
@@ -1286,8 +1282,8 @@ void MediaPlayerService::Client::addNewMetadataUpdate(media::Metadata::Type meta
#if CALLBACK_ANTAGONIZER
const int Antagonizer::interval = 10000; // 10 msecs

Antagonizer::Antagonizer(notify_callback_f cb, void* client) :
    mExit(false), mActive(false), mClient(client), mCb(cb)
Antagonizer::Antagonizer(const sp<MediaPlayerBase::Listener> &listener) :
    mExit(false), mActive(false), mListener(listener)
{
    createThread(callbackThread, this);
}
@@ -1307,7 +1303,7 @@ int Antagonizer::callbackThread(void* user)
    while (!p->mExit) {
        if (p->mActive) {
            ALOGV("send event");
            p->mCb(p->mClient, 0, 0, 0);
            p->mListener->notify(0, 0, 0, 0);
        }
        usleep(interval);
    }
@@ -1346,7 +1342,7 @@ status_t MediaPlayerService::decode(

    // create the right type of player
    sp<AudioCache> cache = new AudioCache(heap);
    player = MediaPlayerFactory::createPlayer(playerType, cache.get(), cache->notify);
    player = MediaPlayerFactory::createPlayer(playerType, cache->getListener());
    if (player == NULL) goto Exit;
    if (player->hardwareOutput()) goto Exit;

@@ -1401,7 +1397,7 @@ status_t MediaPlayerService::decode(int fd, int64_t offset, int64_t length,

    // create the right type of player
    sp<AudioCache> cache = new AudioCache(heap);
    player = MediaPlayerFactory::createPlayer(playerType, cache.get(), cache->notify);
    player = MediaPlayerFactory::createPlayer(playerType, cache->getListener());
    if (player == NULL) goto Exit;
    if (player->hardwareOutput()) goto Exit;

@@ -1996,6 +1992,7 @@ MediaPlayerService::AudioCache::AudioCache(const sp<IMemoryHeap>& heap) :
    mHeap(heap), mChannelCount(0), mFrameCount(1024), mSampleRate(0), mSize(0),
    mFrameSize(1), mError(NO_ERROR), mCommandComplete(false)
{
    mListener = new Listener(this);
}

uint32_t MediaPlayerService::AudioCache::latency () const
@@ -2187,10 +2184,9 @@ status_t MediaPlayerService::AudioCache::wait()
}

void MediaPlayerService::AudioCache::notify(
        void* cookie, int msg, int ext1, int ext2, const Parcel* /*obj*/)
        int msg, int ext1, int ext2, const Parcel* /*obj*/)
{
    ALOGV("notify(%p, %d, %d, %d)", cookie, msg, ext1, ext2);
    AudioCache* p = static_cast<AudioCache*>(cookie);
    ALOGV("notify(%d, %d, %d)", msg, ext1, ext2);

    // ignore buffering messages
    switch (msg)
@@ -2210,12 +2206,12 @@ void MediaPlayerService::AudioCache::notify(
    }

    // wake up thread
    Mutex::Autolock lock(p->mLock);
    Mutex::Autolock lock(mLock);
    if (msg == MEDIA_ERROR) {
        p->mError = ext1;
        mError = ext1;
    }
    p->mCommandComplete = true;
    p->mSignal.signal();
    mCommandComplete = true;
    mSignal.signal();
}

int MediaPlayerService::AudioCache::getSessionId() const
+71 −43
Original line number Diff line number Diff line
@@ -46,7 +46,7 @@ class MediaRecorderClient;
#if CALLBACK_ANTAGONIZER
class Antagonizer {
public:
    Antagonizer(notify_callback_f cb, void* client);
    Antagonizer(const sp<MediaPlayerBase::Listener> &listener);
    void start() { mActive = true; }
    void stop() { mActive = false; }
    void kill();
@@ -58,8 +58,7 @@ private:
    Condition                     mCondition;
    bool                          mExit;
    bool                          mActive;
    void*               mClient;
    notify_callback_f   mCb;
    sp<MediaPlayerBase::Listener> mListener;
};
#endif

@@ -230,13 +229,27 @@ class MediaPlayerService : public BnMediaPlayerService

                sp<IMemoryHeap> getHeap() const { return mHeap; }

        static  void            notify(void* cookie, int msg,
                                       int ext1, int ext2, const Parcel *obj);
                sp<MediaPlayerBase::Listener> getListener() { return mListener; }
                void            notify(int msg, int ext1, int ext2, const Parcel *obj);
        virtual status_t        dump(int fd, const Vector<String16>& args) const;

    private:
                                AudioCache();

        class Listener : public MediaPlayerBase::Listener {
        public:
            Listener(const wp<AudioCache> &client) : mClient(client) {}
            virtual ~Listener() {}
            virtual void notify(int msg, int ext1, int ext2, const Parcel *obj) {
                sp<AudioCache> client = mClient.promote();
                if (client != NULL) {
                    client->notify(msg, ext1, ext2, obj);
                }
            }
        private:
            wp<AudioCache> mClient;
        };

        Mutex                         mLock;
        Condition                     mSignal;
        sp<IMemoryHeap>               mHeap;
@@ -249,6 +262,7 @@ class MediaPlayerService : public BnMediaPlayerService
        size_t                        mFrameSize;
        int                           mError;
        bool                          mCommandComplete;
        sp<MediaPlayerBase::Listener> mListener;

        sp<Thread>                    mCallbackThread;
    }; // AudioCache
@@ -380,8 +394,7 @@ private:
        void                    setDataSource_post(const sp<MediaPlayerBase>& p,
                                                   status_t status);

        static  void            notify(void* cookie, int msg,
                                       int ext1, int ext2, const Parcel *obj);
                void            notify(int msg, int ext1, int ext2, const Parcel *obj);

                pid_t           pid() const { return mPid; }
        virtual status_t        dump(int fd, const Vector<String16>& args) const;
@@ -420,6 +433,20 @@ private:

        status_t setAudioAttributes_l(const Parcel &request);

        class Listener : public MediaPlayerBase::Listener {
        public:
            Listener(const wp<Client> &client) : mClient(client) {}
            virtual ~Listener() {}
            virtual void notify(int msg, int ext1, int ext2, const Parcel *obj) {
                sp<Client> client = mClient.promote();
                if (client != NULL) {
                    client->notify(msg, ext1, ext2, obj);
                }
            }
        private:
            wp<Client> mClient;
        };

        mutable     Mutex                         mLock;
                    sp<MediaPlayerBase>           mPlayer;
                    sp<MediaPlayerService>        mService;
@@ -437,6 +464,7 @@ private:
                    struct sockaddr_in            mRetransmitEndpoint;
                    bool                          mRetransmitEndpointValid;
                    sp<Client>                    mNextClient;
                    sp<MediaPlayerBase::Listener> mListener;

        // Metadata filters.
        media::Metadata::Filter mMetadataAllow;  // protected by mLock