From f12ed30fc4a3f5dd04d7f4999156cff200682060 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Mon, 1 Apr 2019 16:18:53 -0700 Subject: [PATCH 1/3] NuPlayerCCDecoder: fix memory OOB Test: cts Bug: 129068792 Change-Id: Id78ddc983f245feda3a81da3448196340b57f5c9 (cherry picked from commit e1c7348e1c3fed25c16ae4673101f48b1ed95b7e) (cherry picked from commit 0f7ff70737d58abda69fa6d4524b1943d6c41461) --- .../nuplayer/NuPlayerCCDecoder.cpp | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerCCDecoder.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerCCDecoder.cpp index fb12360026..7b2b6fb782 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerCCDecoder.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerCCDecoder.cpp @@ -316,6 +316,11 @@ bool NuPlayer::CCDecoder::extractFromMPEGUserData(const sp &accessUnit) const size_t *userData = (size_t *)mpegUserData->data(); for (size_t i = 0; i < mpegUserData->size() / sizeof(size_t); ++i) { + if (accessUnit->size() < userData[i]) { + ALOGW("b/129068792, skip invalid offset for user data"); + android_errorWriteLog(0x534e4554, "129068792"); + continue; + } trackAdded |= parseMPEGUserDataUnit( timeUs, accessUnit->data() + userData[i], accessUnit->size() - userData[i]); } @@ -325,6 +330,12 @@ bool NuPlayer::CCDecoder::extractFromMPEGUserData(const sp &accessUnit) // returns true if a new CC track is found bool NuPlayer::CCDecoder::parseMPEGUserDataUnit(int64_t timeUs, const uint8_t *data, size_t size) { + if (size < 9) { + ALOGW("b/129068792, MPEG user data size too small %zu", size); + android_errorWriteLog(0x534e4554, "129068792"); + return false; + } + ABitReader br(data + 4, 5); uint32_t user_identifier = br.getBits(32); @@ -377,8 +388,14 @@ bool NuPlayer::CCDecoder::parseMPEGCCData(int64_t timeUs, const uint8_t *data, s mDTVCCPacket->setRange(0, mDTVCCPacket->size() + 2); br.skipBits(16); } else if (mDTVCCPacket->size() > 0 && cc_type == 2) { - memcpy(mDTVCCPacket->data() + mDTVCCPacket->size(), br.data(), 2); - mDTVCCPacket->setRange(0, mDTVCCPacket->size() + 2); + if (mDTVCCPacket->capacity() - mDTVCCPacket->size() >= 2) { + memcpy(mDTVCCPacket->data() + mDTVCCPacket->size(), br.data(), 2); + mDTVCCPacket->setRange(0, mDTVCCPacket->size() + 2); + } else { + ALOGW("b/129068792, skip CC due to too much data(%zu, %zu)", + mDTVCCPacket->capacity(), mDTVCCPacket->size()); + android_errorWriteLog(0x534e4554, "129068792"); + } br.skipBits(16); } else if (cc_type == 0 || cc_type == 1) { uint8_t cc_data_1 = br.getBits(8) & 0x7f; @@ -465,6 +482,11 @@ bool NuPlayer::CCDecoder::parseDTVCCPacket(int64_t timeUs, const uint8_t *data, size_t trackIndex = getTrackIndex(kTrackTypeCEA708, service_number, &trackAdded); if (mSelectedTrack == (ssize_t)trackIndex) { sp ccPacket = new ABuffer(block_size); + if (ccPacket->capacity() == 0) { + ALOGW("b/129068792, no memory available, %zu", block_size); + android_errorWriteLog(0x534e4554, "129068792"); + return false; + } memcpy(ccPacket->data(), br.data(), block_size); mCCMap.add(timeUs, ccPacket); } From 6d37377266de6e43d5ca7658bf2f8f58aa88dcbe Mon Sep 17 00:00:00 2001 From: Weiyin Jiang Date: Fri, 27 Apr 2018 00:39:29 +0800 Subject: [PATCH 2/3] audio: ensure effect chain with specific session id is unique It's possible that tracks with the same session id running on various playback outputs, which causes effect chain being created on the same session twice. As a result, the same effect engine will be released twice as the same context is reused. Output that has effect chain with same session id is more preferable. Test: No regression with Play Music and Effects Bug: 123082420 Bug: 123237974 Merged-In: I690ea3cb942d1fdc96b46048e271557d48000f43 Change-Id: I690ea3cb942d1fdc96b46048e271557d48000f43 (cherry picked from commit 9aeb1770d49bab13ea5c6454c969a713641fe686) (cherry picked from commit 5945746bcabff8d833229a6c230cbe873474087f) --- services/audioflinger/AudioFlinger.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index 687b5a65f5..bc1654e674 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -3102,9 +3102,13 @@ sp AudioFlinger::createEffect( } // look for the thread where the specified audio session is present for (size_t i = 0; i < mPlaybackThreads.size(); i++) { - if (mPlaybackThreads.valueAt(i)->hasAudioSession(sessionId) != 0) { + uint32_t sessionType = mPlaybackThreads.valueAt(i)->hasAudioSession(sessionId); + if (sessionType != 0) { io = mPlaybackThreads.keyAt(i); - break; + // thread with same effect session is preferable + if ((sessionType & ThreadBase::EFFECT_SESSION) != 0) { + break; + } } } if (io == AUDIO_IO_HANDLE_NONE) { From 1aa4d4e7a9496efd19ca891cbff9d71a7c7c0a1f Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Tue, 12 Mar 2019 19:39:03 -0700 Subject: [PATCH 3/3] AudioFlinger: Prevent multiple effect chains with same sessionId Allow at most one effect chain with same sessionId on mPlaybackThreads. Test: poc, CTS effect tests Bug: 123237974 Merged-In: Ide46cd23b0a9f4295f0dca2fea23379a76b836ee Change-Id: Ide46cd23b0a9f4295f0dca2fea23379a76b836ee (cherry picked from commit 1631f06feb36df5406ad00e850dcca9394f67772) (cherry picked from commit f963b2bfdaf406b42d371322402172b4380bbba5) --- services/audioflinger/AudioFlinger.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index bc1654e674..c77d082cfe 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -3134,6 +3134,21 @@ sp AudioFlinger::createEffect( io = mPlaybackThreads.keyAt(0); } ALOGV("createEffect() got io %d for effect %s", io, desc.name); + } else if (checkPlaybackThread_l(io) != nullptr) { + // allow only one effect chain per sessionId on mPlaybackThreads. + for (size_t i = 0; i < mPlaybackThreads.size(); i++) { + const audio_io_handle_t checkIo = mPlaybackThreads.keyAt(i); + if (io == checkIo) continue; + const uint32_t sessionType = + mPlaybackThreads.valueAt(i)->hasAudioSession(sessionId); + if ((sessionType & ThreadBase::EFFECT_SESSION) != 0) { + ALOGE("%s: effect %s io %d denied because session %d effect exists on io %d", + __func__, desc.name, (int)io, (int)sessionId, (int)checkIo); + android_errorWriteLog(0x534e4554, "123237974"); + lStatus = BAD_VALUE; + goto Exit; + } + } } ThreadBase *thread = checkRecordThread_l(io); if (thread == NULL) {