From 4f236c532039a61f0cf681d2e3c6e022911bbb5c Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Thu, 28 Apr 2016 13:32:41 -0700 Subject: Check section size when verifying CRC Bug: 28333006 Change-Id: Ief7a2da848face78f0edde21e2f2009316076679 --- media/libstagefright/mpeg2ts/ATSParser.cpp | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'media') diff --git a/media/libstagefright/mpeg2ts/ATSParser.cpp b/media/libstagefright/mpeg2ts/ATSParser.cpp index e3c3e80..2f2b115 100644 --- a/media/libstagefright/mpeg2ts/ATSParser.cpp +++ b/media/libstagefright/mpeg2ts/ATSParser.cpp @@ -1713,6 +1713,13 @@ bool ATSParser::PSISection::isCRCOkay() const { unsigned sectionLength = U16_AT(data + 1) & 0xfff; ALOGV("sectionLength %u, skip %u", sectionLength, mSkipBytes); + + if(sectionLength < mSkipBytes) { + ALOGE("b/28333006"); + android_errorWriteLog(0x534e4554, "28333006"); + return false; + } + // Skip the preceding field present when payload start indicator is on. sectionLength -= mSkipBytes; -- cgit v1.1 From daef4327fe0c75b0a90bb8627458feec7a301e1f Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Mon, 2 May 2016 14:12:34 -0700 Subject: Clear unused pointer field when sending across binder Bug: 28377502 Change-Id: Iad5ebfb0a9ef89f09755bb332579dbd3534f9c98 --- media/libmediaplayerservice/MetadataRetrieverClient.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'media') diff --git a/media/libmediaplayerservice/MetadataRetrieverClient.cpp b/media/libmediaplayerservice/MetadataRetrieverClient.cpp index a5a1fa5..f6acdf6 100644 --- a/media/libmediaplayerservice/MetadataRetrieverClient.cpp +++ b/media/libmediaplayerservice/MetadataRetrieverClient.cpp @@ -231,6 +231,7 @@ sp MetadataRetrieverClient::getFrameAtTime(int64_t timeUs, int option) ALOGV("rotation: %d", frameCopy->mRotationAngle); frameCopy->mData = (uint8_t *)frameCopy + sizeof(VideoFrame); memcpy(frameCopy->mData, frame->mData, frame->mSize); + frameCopy->mData = 0; delete frame; // Fix memory leakage return mThumbnail; } -- cgit v1.1 From 60547808ca4e9cfac50028c00c58a6ceb2319301 Mon Sep 17 00:00:00 2001 From: Pawin Vongmasa Date: Wed, 11 May 2016 16:08:21 -0700 Subject: h264bsdActivateParamSets: Prevent multiplication overflow. Report MEMORY_ALLOCATION_ERROR if pStorage->picSizeInMbs would exceed UINT32_MAX bytes. Bug: 28532266 Change-Id: Ia6f11efb18818afcdb5fa2a38a14f2a2d8c8447a --- .../codecs/on2/h264dec/source/h264bsd_storage.c | 24 +++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) (limited to 'media') diff --git a/media/libstagefright/codecs/on2/h264dec/source/h264bsd_storage.c b/media/libstagefright/codecs/on2/h264dec/source/h264bsd_storage.c index 3234754..ff7a42a 100644 --- a/media/libstagefright/codecs/on2/h264dec/source/h264bsd_storage.c +++ b/media/libstagefright/codecs/on2/h264dec/source/h264bsd_storage.c @@ -58,6 +58,10 @@ 3. Module defines ------------------------------------------------------------------------------*/ +#ifndef UINT32_MAX +#define UINT32_MAX (4294967295U) +#endif + /*------------------------------------------------------------------------------ 4. Local function prototypes ------------------------------------------------------------------------------*/ @@ -326,9 +330,23 @@ u32 h264bsdActivateParamSets(storage_t *pStorage, u32 ppsId, u32 isIdr) pStorage->activePps = pStorage->pps[ppsId]; pStorage->activeSpsId = pStorage->activePps->seqParameterSetId; pStorage->activeSps = pStorage->sps[pStorage->activeSpsId]; - pStorage->picSizeInMbs = - pStorage->activeSps->picWidthInMbs * - pStorage->activeSps->picHeightInMbs; + + /* report error before multiplication to prevent integer overflow */ + if (pStorage->activeSps->picWidthInMbs == 0) + { + pStorage->picSizeInMbs = 0; + } + else if (pStorage->activeSps->picHeightInMbs > + UINT32_MAX / pStorage->activeSps->picWidthInMbs) + { + return(MEMORY_ALLOCATION_ERROR); + } + else + { + pStorage->picSizeInMbs = + pStorage->activeSps->picWidthInMbs * + pStorage->activeSps->picHeightInMbs; + } pStorage->currImage->width = pStorage->activeSps->picWidthInMbs; pStorage->currImage->height = pStorage->activeSps->picHeightInMbs; -- cgit v1.1 From e248db02fbab2ee9162940bc19f087fd7d96cb9d Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Fri, 13 May 2016 11:48:11 -0700 Subject: Fix security vulnerability in libstagefright bug: 28175045 Change-Id: Icee6c7eb5b761da4aa3e412fb71825508d74d38f --- media/libstagefright/DRMExtractor.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libstagefright/DRMExtractor.cpp b/media/libstagefright/DRMExtractor.cpp index 9cb6e86..e2bc89c 100644 --- a/media/libstagefright/DRMExtractor.cpp +++ b/media/libstagefright/DRMExtractor.cpp @@ -200,7 +200,17 @@ status_t DRMSource::read(MediaBuffer **buffer, const ReadOptions *options) { continue; } - CHECK(dstOffset + 4 <= (*buffer)->size()); + if (dstOffset > SIZE_MAX - 4 || + dstOffset + 4 > SIZE_MAX - nalLength || + dstOffset + 4 + nalLength > (*buffer)->size()) { + (*buffer)->release(); + (*buffer) = NULL; + if (decryptedDrmBuffer.data) { + delete [] decryptedDrmBuffer.data; + decryptedDrmBuffer.data = NULL; + } + return ERROR_MALFORMED; + } dstData[dstOffset++] = 0; dstData[dstOffset++] = 0; -- cgit v1.1 From e7142a0703bc93f75e213e96ebc19000022afed9 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Wed, 11 May 2016 11:11:20 -0700 Subject: Check malloc result to avoid NPD Bug: 28471206 Change-Id: Id5d055d76893d6f53a2e524ff5f282d1ddca3345 --- media/libstagefright/MPEG4Extractor.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'media') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index f8789da..f6206d2 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -535,6 +535,10 @@ status_t MPEG4Extractor::readMetaData() { } if (psshsize > 0 && psshsize <= UINT32_MAX) { char *buf = (char*)malloc(psshsize); + if (!buf) { + ALOGE("b/28471206"); + return NO_MEMORY; + } char *ptr = buf; for (size_t i = 0; i < mPssh.size(); i++) { memcpy(ptr, mPssh[i].uuid, 20); // uuid + length @@ -1702,6 +1706,11 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { sp buffer = new ABuffer(chunk_data_size); + if (buffer->data() == NULL) { + ALOGE("b/28471206"); + return NO_MEMORY; + } + if (mDataSource->readAt( data_offset, buffer->data(), chunk_data_size) < chunk_data_size) { return ERROR_IO; @@ -1719,6 +1728,11 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { { sp buffer = new ABuffer(chunk_data_size); + if (buffer->data() == NULL) { + ALOGE("b/28471206"); + return NO_MEMORY; + } + if (mDataSource->readAt( data_offset, buffer->data(), chunk_data_size) < chunk_data_size) { return ERROR_IO; @@ -2051,6 +2065,10 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { return ERROR_MALFORMED; } sp buffer = new ABuffer(chunk_data_size + 1); + if (buffer->data() == NULL) { + ALOGE("b/28471206"); + return NO_MEMORY; + } if (mDataSource->readAt( data_offset, buffer->data(), chunk_data_size) != (ssize_t)chunk_data_size) { return ERROR_IO; -- cgit v1.1 From 6fdee2a83432b3b150d6a34f231c4e2f7353c01e Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Fri, 13 May 2016 10:39:23 -0700 Subject: limit mediaserver memory Limit mediaserver using rlimit, to prevent it from bringing down the system via the low memory killer. Default max is 65% of total RAM, but can be customized via system property. Bug: 28471206 Bug: 28615448 Change-Id: Ic84137435d1ef0a6883e9789a4b4f399e4283f05 --- media/libmedia/Android.mk | 1 + media/libmedia/MediaUtils.cpp | 74 ++++++++++++++++++++++++++++++++++ media/libmedia/MediaUtils.h | 35 ++++++++++++++++ media/mediaserver/Android.mk | 1 + media/mediaserver/main_mediaserver.cpp | 6 +++ 5 files changed, 117 insertions(+) create mode 100644 media/libmedia/MediaUtils.cpp create mode 100644 media/libmedia/MediaUtils.h (limited to 'media') diff --git a/media/libmedia/Android.mk b/media/libmedia/Android.mk index a3c3d3c..9f836f0 100644 --- a/media/libmedia/Android.mk +++ b/media/libmedia/Android.mk @@ -44,6 +44,7 @@ LOCAL_SRC_FILES:= \ IResourceManagerService.cpp \ IStreamSource.cpp \ MediaCodecInfo.cpp \ + MediaUtils.cpp \ Metadata.cpp \ mediarecorder.cpp \ IMediaMetadataRetriever.cpp \ diff --git a/media/libmedia/MediaUtils.cpp b/media/libmedia/MediaUtils.cpp new file mode 100644 index 0000000..a02ca65 --- /dev/null +++ b/media/libmedia/MediaUtils.cpp @@ -0,0 +1,74 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "MediaUtils" +#define LOG_NDEBUG 0 +#include + +#include +#include +#include + +#include "MediaUtils.h" + +namespace android { + +void limitProcessMemory( + const char *property, + size_t numberOfBytes, + size_t percentageOfTotalMem) { + + long pageSize = sysconf(_SC_PAGESIZE); + long numPages = sysconf(_SC_PHYS_PAGES); + size_t maxMem = SIZE_MAX; + + if (pageSize > 0 && numPages > 0) { + if (size_t(numPages) < SIZE_MAX / size_t(pageSize)) { + maxMem = size_t(numPages) * size_t(pageSize); + } + ALOGV("physMem: %zu", maxMem); + if (percentageOfTotalMem > 100) { + ALOGW("requested %zu%% of total memory, using 100%%", percentageOfTotalMem); + percentageOfTotalMem = 100; + } + maxMem = maxMem / 100 * percentageOfTotalMem; + if (numberOfBytes < maxMem) { + maxMem = numberOfBytes; + } + ALOGV("requested limit: %zu", maxMem); + } else { + ALOGW("couldn't determine total RAM"); + } + + int64_t propVal = property_get_int64(property, maxMem); + if (propVal > 0 && uint64_t(propVal) <= SIZE_MAX) { + maxMem = propVal; + } + ALOGV("actual limit: %zu", maxMem); + + struct rlimit limit; + getrlimit(RLIMIT_AS, &limit); + ALOGV("original limits: %lld/%lld", (long long)limit.rlim_cur, (long long)limit.rlim_max); + limit.rlim_cur = maxMem; + setrlimit(RLIMIT_AS, &limit); + limit.rlim_cur = -1; + limit.rlim_max = -1; + getrlimit(RLIMIT_AS, &limit); + ALOGV("new limits: %lld/%lld", (long long)limit.rlim_cur, (long long)limit.rlim_max); + +} + +} // namespace android diff --git a/media/libmedia/MediaUtils.h b/media/libmedia/MediaUtils.h new file mode 100644 index 0000000..f80dd30 --- /dev/null +++ b/media/libmedia/MediaUtils.h @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef _MEDIA_UTILS_H +#define _MEDIA_UTILS_H + +namespace android { + +/** + Limit the amount of memory a process can allocate using setrlimit(RLIMIT_AS). + The value to use will be read from the specified system property, or if the + property doesn't exist it will use the specified number of bytes or the + specified percentage of total memory, whichever is smaller. +*/ +void limitProcessMemory( + const char *property, + size_t numberOfBytes, + size_t percentageOfTotalMem); + +} // namespace android + +#endif // _MEDIA_UTILS_H diff --git a/media/mediaserver/Android.mk b/media/mediaserver/Android.mk index b6de0d9..7e017b9 100644 --- a/media/mediaserver/Android.mk +++ b/media/mediaserver/Android.mk @@ -37,6 +37,7 @@ LOCAL_STATIC_LIBRARIES := \ LOCAL_C_INCLUDES := \ frameworks/av/media/libmediaplayerservice \ + frameworks/av/media/libmedia \ frameworks/av/services/medialog \ frameworks/av/services/audioflinger \ frameworks/av/services/audiopolicy \ diff --git a/media/mediaserver/main_mediaserver.cpp b/media/mediaserver/main_mediaserver.cpp index 4a485ed..8cc9508 100644 --- a/media/mediaserver/main_mediaserver.cpp +++ b/media/mediaserver/main_mediaserver.cpp @@ -36,6 +36,7 @@ #include "MediaPlayerService.h" #include "ResourceManagerService.h" #include "service/AudioPolicyService.h" +#include "MediaUtils.h" #include "SoundTriggerHwService.h" #include "RadioService.h" @@ -43,6 +44,11 @@ using namespace android; int main(int argc __unused, char** argv) { + limitProcessMemory( + "ro.media.maxmem", /* property that defines limit */ + SIZE_MAX, /* upper limit in bytes */ + 65 /* upper limit as percentage of physical RAM */); + signal(SIGPIPE, SIG_IGN); char value[PROPERTY_VALUE_MAX]; bool doLog = (property_get("ro.test_harness", value, "0") > 0) && (atoi(value) == 1); -- cgit v1.1 From f81038006b4c59a5a148dcad887371206033c28f Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Fri, 28 Aug 2015 10:35:35 -0700 Subject: MPEG4Extractor: ensure kKeyTrackID exists before creating an MPEG4Source as track. GenericSource: return error when no track exists. SampleIterator: make sure mSamplesPerChunk is not zero before using it as divisor. Bug: 21657957 Bug: 23705695 Bug: 22802344 Bug: 28799341 Change-Id: I7664992ade90b935d3f255dcd43ecc2898f30b04 (cherry picked from commit 0386c91b8a910a134e5898ffa924c1b6c7560b13) --- .../nuplayer/GenericSource.cpp | 34 +++++++++++++--------- media/libstagefright/MPEG4Extractor.cpp | 8 +++++ media/libstagefright/SampleIterator.cpp | 5 ++++ 3 files changed, 33 insertions(+), 14 deletions(-) (limited to 'media') diff --git a/media/libmediaplayerservice/nuplayer/GenericSource.cpp b/media/libmediaplayerservice/nuplayer/GenericSource.cpp index 45da218..0598254 100644 --- a/media/libmediaplayerservice/nuplayer/GenericSource.cpp +++ b/media/libmediaplayerservice/nuplayer/GenericSource.cpp @@ -211,6 +211,9 @@ status_t NuPlayer::GenericSource::initFromDataSource() { for (size_t i = 0; i < numtracks; ++i) { sp track = extractor->getTrack(i); + if (track == NULL) { + continue; + } sp meta = extractor->getTrackMetaData(i); @@ -253,24 +256,27 @@ status_t NuPlayer::GenericSource::initFromDataSource() { } } - if (track != NULL) { - mSources.push(track); - int64_t durationUs; - if (meta->findInt64(kKeyDuration, &durationUs)) { - if (durationUs > mDurationUs) { - mDurationUs = durationUs; - } + mSources.push(track); + int64_t durationUs; + if (meta->findInt64(kKeyDuration, &durationUs)) { + if (durationUs > mDurationUs) { + mDurationUs = durationUs; } + } - int32_t bitrate; - if (totalBitrate >= 0 && meta->findInt32(kKeyBitRate, &bitrate)) { - totalBitrate += bitrate; - } else { - totalBitrate = -1; - } + int32_t bitrate; + if (totalBitrate >= 0 && meta->findInt32(kKeyBitRate, &bitrate)) { + totalBitrate += bitrate; + } else { + totalBitrate = -1; } } + if (mSources.size() == 0) { + ALOGE("b/23705695"); + return UNKNOWN_ERROR; + } + mBitrate = totalBitrate; return OK; @@ -318,7 +324,7 @@ int64_t NuPlayer::GenericSource::getLastReadPosition() { status_t NuPlayer::GenericSource::setBuffers( bool audio, Vector &buffers) { - if (mIsSecure && !audio) { + if (mIsWidevine && !audio && mVideoTrack.mSource != NULL) { return mVideoTrack.mSource->setBuffers(buffers); } return INVALID_OPERATION; diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index f6206d2..4c10cc9 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -933,6 +933,11 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { } if (isTrack) { + int32_t trackId; + // There must be exact one track header per track. + if (!mLastTrack->meta->findInt32(kKeyTrackID, &trackId)) { + mLastTrack->skipTrack = true; + } if (mLastTrack->skipTrack) { Track *cur = mFirstTrack; @@ -2869,6 +2874,9 @@ sp MPEG4Extractor::getTrack(size_t index) { break; } } + } else { + ALOGE("b/21657957"); + return NULL; } ALOGV("getTrack called, pssh: %zu", mPssh.size()); diff --git a/media/libstagefright/SampleIterator.cpp b/media/libstagefright/SampleIterator.cpp index 2748349..c5f359e 100644 --- a/media/libstagefright/SampleIterator.cpp +++ b/media/libstagefright/SampleIterator.cpp @@ -84,6 +84,11 @@ status_t SampleIterator::seekTo(uint32_t sampleIndex) { CHECK(sampleIndex < mStopChunkSampleIndex); + if (mSamplesPerChunk == 0) { + ALOGE("b/22802344"); + return ERROR_MALFORMED; + } + uint32_t chunk = (sampleIndex - mFirstChunkSampleIndex) / mSamplesPerChunk + mFirstChunk; -- cgit v1.1 From d112f7d0c1dbaf0368365885becb11ca8d3f13a4 Mon Sep 17 00:00:00 2001 From: Dave Weinstein Date: Tue, 14 Jun 2016 11:05:01 -0700 Subject: Resolve a merge issue between lmp and lmp-mr1+ Change-Id: I336cb003fb7f50fd7d95c30ca47e45530a7ad503 (cherry picked from commit 33f6da1092834f1e4be199cfa3b6310d66b521c0) (cherry picked from commit bb3a0338b58fafb01ac5b34efc450b80747e71e4) --- media/libmediaplayerservice/nuplayer/GenericSource.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmediaplayerservice/nuplayer/GenericSource.cpp b/media/libmediaplayerservice/nuplayer/GenericSource.cpp index 0598254..e8c28d5 100644 --- a/media/libmediaplayerservice/nuplayer/GenericSource.cpp +++ b/media/libmediaplayerservice/nuplayer/GenericSource.cpp @@ -324,7 +324,7 @@ int64_t NuPlayer::GenericSource::getLastReadPosition() { status_t NuPlayer::GenericSource::setBuffers( bool audio, Vector &buffers) { - if (mIsWidevine && !audio && mVideoTrack.mSource != NULL) { + if (mIsSecure && !audio && mVideoTrack.mSource != NULL) { return mVideoTrack.mSource->setBuffers(buffers); } return INVALID_OPERATION; -- cgit v1.1