From b52c75787d068a92c961d29f973e975b7651abe5 Mon Sep 17 00:00:00 2001 From: Pawin Vongmasa Date: Mon, 18 Jul 2016 20:12:02 -0700 Subject: SoftMPEG4: Check the buffer size before writing the reference frame. Also prevent overflow in SoftMPEG4 and division by zero in SoftMPEG4Encoder. Bug: 30033990 Change-Id: I7701f5fc54c2670587d122330e5dc851f64ed3c2 (cherry picked from commit 695123195034402ca76169b195069c28c30342d3) --- .../codecs/m4v_h263/dec/SoftMPEG4.cpp | 22 ++++++++++++++++++++-- .../codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp | 4 ++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/media/libstagefright/codecs/m4v_h263/dec/SoftMPEG4.cpp b/media/libstagefright/codecs/m4v_h263/dec/SoftMPEG4.cpp index bb59ae4..e1cfc06 100644 --- a/media/libstagefright/codecs/m4v_h263/dec/SoftMPEG4.cpp +++ b/media/libstagefright/codecs/m4v_h263/dec/SoftMPEG4.cpp @@ -210,8 +210,17 @@ void SoftMPEG4::onQueueFilled(OMX_U32 /* portIndex */) { PortInfo *port = editPortInfo(1); OMX_BUFFERHEADERTYPE *outHeader = port->mBuffers.editItemAt(1).mHeader; + OMX_U32 yFrameSize = sizeof(uint8) * mHandle->size; + if ((outHeader->nAllocLen < yFrameSize) || + (outHeader->nAllocLen - yFrameSize < yFrameSize / 2)) { + ALOGE("Too small output buffer for reference frame: %zu bytes", + outHeader->nAllocLen); + android_errorWriteLog(0x534e4554, "30033990"); + notify(OMX_EventError, OMX_ErrorUndefined, 0, NULL); + mSignalledError = true; + return; + } PVSetReferenceYUV(mHandle, outHeader->pBuffer); - mFramesConfigured = true; } @@ -229,7 +238,16 @@ void SoftMPEG4::onQueueFilled(OMX_U32 /* portIndex */) { int32_t bufferSize = inHeader->nFilledLen; int32_t tmp = bufferSize; - OMX_U32 frameSize = (mWidth * mHeight * 3) / 2; + OMX_U32 frameSize; + OMX_U64 yFrameSize = (OMX_U64)mWidth * (OMX_U64)mHeight; + if (yFrameSize > ((OMX_U64)UINT32_MAX / 3) * 2) { + ALOGE("Frame size too large"); + notify(OMX_EventError, OMX_ErrorUndefined, 0, NULL); + mSignalledError = true; + return; + } + frameSize = (OMX_U32)(yFrameSize + (yFrameSize / 2)); + if (outHeader->nAllocLen < frameSize) { android_errorWriteLog(0x534e4554, "27833616"); ALOGE("Insufficient output buffer size"); diff --git a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp index 7638bb7..2eb51c9 100644 --- a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp +++ b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp @@ -116,6 +116,10 @@ OMX_ERRORTYPE SoftMPEG4Encoder::initEncParams() { ALOGE("Failed to get default encoding parameters"); return OMX_ErrorUndefined; } + if (mFramerate == 0) { + ALOGE("Framerate should not be 0"); + return OMX_ErrorUndefined; + } mEncParams->encMode = mEncodeMode; mEncParams->encWidth[0] = mWidth; mEncParams->encHeight[0] = mHeight; -- cgit v1.1 From 173e6eb58c8df2b934a5602732fe0b0aac1cd03f Mon Sep 17 00:00:00 2001 From: Ray Essick Date: Tue, 16 Aug 2016 14:24:43 -0700 Subject: better validation lengths of strings in ID3 tags Validate lengths on strings in ID3 tags, particularly around 0. Also added code to handle cases when we can't get memory for copies of strings we want to extract from these tags. Affects L/M/N/master, same patch for all of them. Bug: 30744884 Change-Id: I2675a817a39f0927ec1f7e9f9c09f2e61020311e Test: play mp3 file which caused a <0 length. (cherry picked from commit d23c01546c4f82840a01a380def76ab6cae5d43f) --- media/libstagefright/id3/ID3.cpp | 57 +++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/media/libstagefright/id3/ID3.cpp b/media/libstagefright/id3/ID3.cpp index 4410579..6e14197 100644 --- a/media/libstagefright/id3/ID3.cpp +++ b/media/libstagefright/id3/ID3.cpp @@ -77,7 +77,10 @@ ID3::ID3(const uint8_t *data, size_t size, bool ignoreV1) mFirstFrameOffset(0), mVersion(ID3_UNKNOWN), mRawSize(0) { - sp source = new MemorySource(data, size); + sp source = new (std::nothrow) MemorySource(data, size); + + if (source == NULL) + return; mIsValid = parseV2(source, 0); @@ -542,6 +545,10 @@ void ID3::Iterator::getstring(String8 *id, bool otherdata) const { n -= skipped; } + if (n <= 0) { + return; + } + if (encoding == 0x00) { // supposedly ISO 8859-1 id->setTo((const char*)frameData + 1, n); @@ -555,11 +562,16 @@ void ID3::Iterator::getstring(String8 *id, bool otherdata) const { const char16_t *framedata = (const char16_t *) (frameData + 1); char16_t *framedatacopy = NULL; #if BYTE_ORDER == LITTLE_ENDIAN - framedatacopy = new char16_t[len]; - for (int i = 0; i < len; i++) { - framedatacopy[i] = bswap_16(framedata[i]); + if (len > 0) { + framedatacopy = new (std::nothrow) char16_t[len]; + if (framedatacopy == NULL) { + return; + } + for (int i = 0; i < len; i++) { + framedatacopy[i] = bswap_16(framedata[i]); + } + framedata = framedatacopy; } - framedata = framedatacopy; #endif id->setTo(framedata, len); if (framedatacopy != NULL) { @@ -572,15 +584,26 @@ void ID3::Iterator::getstring(String8 *id, bool otherdata) const { const char16_t *framedata = (const char16_t *) (frameData + 1); char16_t *framedatacopy = NULL; if (*framedata == 0xfffe) { - // endianness marker doesn't match host endianness, convert - framedatacopy = new char16_t[len]; + // endianness marker != host endianness, convert & skip + if (len <= 1) { + return; // nothing after the marker + } + framedatacopy = new (std::nothrow) char16_t[len]; + if (framedatacopy == NULL) { + return; + } for (int i = 0; i < len; i++) { framedatacopy[i] = bswap_16(framedata[i]); } framedata = framedatacopy; - } - // If the string starts with an endianness marker, skip it - if (*framedata == 0xfeff) { + // and skip over the marker + framedata++; + len--; + } else if (*framedata == 0xfeff) { + // endianness marker == host endianness, skip it + if (len <= 1) { + return; // nothing after the marker + } framedata++; len--; } @@ -595,12 +618,16 @@ void ID3::Iterator::getstring(String8 *id, bool otherdata) const { } if (eightBit) { // collapse to 8 bit, then let the media scanner client figure out the real encoding - char *frame8 = new char[len]; - for (int i = 0; i < len; i++) { - frame8[i] = framedata[i]; + char *frame8 = new (std::nothrow) char[len]; + if (frame8 != NULL) { + for (int i = 0; i < len; i++) { + frame8[i] = framedata[i]; + } + id->setTo(frame8, len); + delete [] frame8; + } else { + id->setTo(framedata, len); } - id->setTo(frame8, len); - delete [] frame8; } else { id->setTo(framedata, len); } -- cgit v1.1 From 54ac34642df5b547afef19b94f6cc9a8062a49b5 Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Tue, 16 Aug 2016 16:50:54 -0700 Subject: MediaPlayerService: avoid invalid static cast Bug: 30204103 Change-Id: Ie0dd3568a375f1e9fed8615ad3d85184bcc99028 (cherry picked from commit ee0a0e39acdcf8f97e0d6945c31ff36a06a36e9d) --- media/libmediaplayerservice/MediaPlayerService.cpp | 10 ++++++++++ media/libmediaplayerservice/MediaPlayerService.h | 1 + 2 files changed, 11 insertions(+) diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp index b8c610d..0483228 100644 --- a/media/libmediaplayerservice/MediaPlayerService.cpp +++ b/media/libmediaplayerservice/MediaPlayerService.cpp @@ -559,6 +559,12 @@ void MediaPlayerService::removeClient(wp client) mClients.remove(client); } +bool MediaPlayerService::hasClient(wp client) +{ + Mutex::Autolock lock(mLock); + return mClients.indexOf(client) != NAME_NOT_FOUND; +} + MediaPlayerService::Client::Client( const sp& service, pid_t pid, int32_t connId, const sp& client, @@ -1056,6 +1062,10 @@ status_t MediaPlayerService::Client::setNextPlayer(const sp& playe ALOGV("setNextPlayer"); Mutex::Autolock l(mLock); sp c = static_cast(player.get()); + if (!mService->hasClient(c)) { + return BAD_VALUE; + } + mNextClient = c; if (c != NULL) { diff --git a/media/libmediaplayerservice/MediaPlayerService.h b/media/libmediaplayerservice/MediaPlayerService.h index 60d4617..e8e5360 100644 --- a/media/libmediaplayerservice/MediaPlayerService.h +++ b/media/libmediaplayerservice/MediaPlayerService.h @@ -226,6 +226,7 @@ public: virtual status_t dump(int fd, const Vector& args); void removeClient(wp client); + bool hasClient(wp client); // For battery usage tracking purpose struct BatteryUsageInfo { -- cgit v1.1 From 4d96096488a9e5b0a65638f93e71b79bee80e760 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Wed, 17 Aug 2016 06:19:32 -0700 Subject: soundtrigger: add size check on sound model and recogntion data Bug: 30148546 Change-Id: I082f535a853c96571887eeea37c6d41ecee7d8c0 (cherry picked from commit bb00d8f139ff51336ab3c810d35685003949bcf8) (cherry picked from commit ef0c91518446e65533ca8bab6726a845f27c73fd) --- services/soundtrigger/SoundTriggerHwService.cpp | 38 +++++++++++++++++++------ 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/services/soundtrigger/SoundTriggerHwService.cpp b/services/soundtrigger/SoundTriggerHwService.cpp index 9de6fe2..e428388 100644 --- a/services/soundtrigger/SoundTriggerHwService.cpp +++ b/services/soundtrigger/SoundTriggerHwService.cpp @@ -534,6 +534,22 @@ status_t SoundTriggerHwService::Module::loadSoundModel(const sp& modelM struct sound_trigger_sound_model *sound_model = (struct sound_trigger_sound_model *)modelMemory->pointer(); + size_t structSize; + if (sound_model->type == SOUND_MODEL_TYPE_KEYPHRASE) { + structSize = sizeof(struct sound_trigger_phrase_sound_model); + } else { + structSize = sizeof(struct sound_trigger_sound_model); + } + + if (sound_model->data_offset < structSize || + sound_model->data_size > (UINT_MAX - sound_model->data_offset) || + modelMemory->size() < sound_model->data_offset || + sound_model->data_size > (modelMemory->size() - sound_model->data_offset)) { + android_errorWriteLog(0x534e4554, "30148546"); + ALOGE("loadSoundModel() data_size is too big"); + return BAD_VALUE; + } + AutoMutex lock(mLock); if (mModels.size() >= mDescriptor.properties.max_sound_models) { @@ -603,11 +619,23 @@ status_t SoundTriggerHwService::Module::startRecognition(sound_model_handle_t ha return PERMISSION_DENIED; } - if (dataMemory != 0 && dataMemory->pointer() == NULL) { - ALOGE("startRecognition() dataMemory is non-0 but has NULL pointer()"); + if (dataMemory == 0 || dataMemory->pointer() == NULL) { + ALOGE("startRecognition() dataMemory is 0 or has NULL pointer()"); return BAD_VALUE; } + + struct sound_trigger_recognition_config *config = + (struct sound_trigger_recognition_config *)dataMemory->pointer(); + + if (config->data_offset < sizeof(struct sound_trigger_recognition_config) || + config->data_size > (UINT_MAX - config->data_offset) || + dataMemory->size() < config->data_offset || + config->data_size > (dataMemory->size() - config->data_offset)) { + ALOGE("startRecognition() data_size is too big"); + return BAD_VALUE; + } + AutoMutex lock(mLock); if (mServiceState == SOUND_TRIGGER_STATE_DISABLED) { return INVALID_OPERATION; @@ -616,17 +644,11 @@ status_t SoundTriggerHwService::Module::startRecognition(sound_model_handle_t ha if (model == 0) { return BAD_VALUE; } - if ((dataMemory == 0) || - (dataMemory->size() < sizeof(struct sound_trigger_recognition_config))) { - return BAD_VALUE; - } if (model->mState == Model::STATE_ACTIVE) { return INVALID_OPERATION; } - struct sound_trigger_recognition_config *config = - (struct sound_trigger_recognition_config *)dataMemory->pointer(); //TODO: get capture handle and device from audio policy service config->capture_handle = model->mCaptureIOHandle; -- cgit v1.1 From 497fcd71dd221dd2ccf6b32127ece5ec43c8660e Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Wed, 17 Aug 2016 14:11:13 -0700 Subject: Add EFFECT_CMD_SET_PARAM parameter checking Bug: 30204301 Change-Id: Ib9c3ee1c2f23c96f8f7092dd9e146bc453d7a290 (cherry picked from commit e4a1d91501d47931dbae19c47815952378787ab6) --- services/audioflinger/Effects.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp index eb52dee..f87b8f5 100644 --- a/services/audioflinger/Effects.cpp +++ b/services/audioflinger/Effects.cpp @@ -543,6 +543,13 @@ status_t AudioFlinger::EffectModule::remove_effect_from_hal_l() return NO_ERROR; } +// round up delta valid if value and divisor are positive. +template +static T roundUpDelta(const T &value, const T &divisor) { + T remainder = value % divisor; + return remainder == 0 ? 0 : divisor - remainder; +} + status_t AudioFlinger::EffectModule::command(uint32_t cmdCode, uint32_t cmdSize, void *pCmdData, @@ -564,6 +571,22 @@ status_t AudioFlinger::EffectModule::command(uint32_t cmdCode, android_errorWriteLog(0x534e4554, "29251553"); return -EINVAL; } + if ((cmdCode == EFFECT_CMD_SET_PARAM + || cmdCode == EFFECT_CMD_SET_PARAM_DEFERRED) && // DEFERRED not generally used + (sizeof(effect_param_t) > cmdSize + || ((effect_param_t *)pCmdData)->psize > cmdSize + - sizeof(effect_param_t) + || ((effect_param_t *)pCmdData)->vsize > cmdSize + - sizeof(effect_param_t) + - ((effect_param_t *)pCmdData)->psize + || roundUpDelta(((effect_param_t *)pCmdData)->psize, (uint32_t)sizeof(int)) > + cmdSize + - sizeof(effect_param_t) + - ((effect_param_t *)pCmdData)->psize + - ((effect_param_t *)pCmdData)->vsize)) { + android_errorWriteLog(0x534e4554, "30204301"); + return -EINVAL; + } status_t status = (*mEffectInterface)->command(mEffectInterface, cmdCode, cmdSize, -- cgit v1.1 From 45f500ac36958b36afa649c8d68490cd38442252 Mon Sep 17 00:00:00 2001 From: Pawin Vongmasa Date: Fri, 19 Aug 2016 02:07:33 -0700 Subject: Fix build breakage caused by commit 940829f69b52d6038db66a9c727534636ecc456d. Change-Id: I4776db4a26fb3c31bb994d48788373fe569c812a (cherry picked from commit baa9146401e28c5acf54dea21ddd197f0d3a8fcd) --- media/libstagefright/codecs/m4v_h263/dec/SoftMPEG4.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/media/libstagefright/codecs/m4v_h263/dec/SoftMPEG4.cpp b/media/libstagefright/codecs/m4v_h263/dec/SoftMPEG4.cpp index e1cfc06..1dd631a 100644 --- a/media/libstagefright/codecs/m4v_h263/dec/SoftMPEG4.cpp +++ b/media/libstagefright/codecs/m4v_h263/dec/SoftMPEG4.cpp @@ -213,8 +213,8 @@ void SoftMPEG4::onQueueFilled(OMX_U32 /* portIndex */) { OMX_U32 yFrameSize = sizeof(uint8) * mHandle->size; if ((outHeader->nAllocLen < yFrameSize) || (outHeader->nAllocLen - yFrameSize < yFrameSize / 2)) { - ALOGE("Too small output buffer for reference frame: %zu bytes", - outHeader->nAllocLen); + ALOGE("Too small output buffer for reference frame: %lu bytes", + (unsigned long)outHeader->nAllocLen); android_errorWriteLog(0x534e4554, "30033990"); notify(OMX_EventError, OMX_ErrorUndefined, 0, NULL); mSignalledError = true; -- cgit v1.1 From 4b459da1ee3bb52ddfcc649dc2686975a9d72ab7 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Tue, 30 Aug 2016 13:49:06 -0700 Subject: MediaPlayerService: allow next player to be NULL Bug: 31155917 Bug: 30204103 Change-Id: I9a2a59ddb900fc942e7c19b31b53a110d790474c (cherry picked from commit 282841278723166e74039329ca56e444ad472daf) --- media/libmediaplayerservice/MediaPlayerService.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp index 0483228..f802de1 100644 --- a/media/libmediaplayerservice/MediaPlayerService.cpp +++ b/media/libmediaplayerservice/MediaPlayerService.cpp @@ -1062,7 +1062,7 @@ status_t MediaPlayerService::Client::setNextPlayer(const sp& playe ALOGV("setNextPlayer"); Mutex::Autolock l(mLock); sp c = static_cast(player.get()); - if (!mService->hasClient(c)) { + if (c != NULL && !mService->hasClient(c)) { return BAD_VALUE; } -- cgit v1.1 From bc8a45f506a8be33250c523d71fab637a5fdaf81 Mon Sep 17 00:00:00 2001 From: Lajos Molnar Date: Tue, 2 Aug 2016 07:07:05 -0700 Subject: IOMX: work against metadata buffer spoofing - Prohibit direct set/getParam/Settings for extensions meant for OMXNodeInstance alone. This disallows enabling metadata mode without the knowledge of OMXNodeInstance. - Do not share metadata mode buffers cross process. - Disallow setting up metadata mode/tunneling/input surface after first sendCommand. - Disallow store-meta for input cross process. - Disallow emptyBuffer for surface input (via IOMX). - Fix checking for input surface. Bug: 29422020 Change-Id: I801c77b80e703903f62e42d76fd2e76a34e4bc8e (cherry picked from commit f8a4cb410115045278f534e54b41ac78d6bf6c07) --- include/media/IOMX.h | 4 +- media/libmedia/IOMX.cpp | 14 +- media/libstagefright/OMXClient.cpp | 13 +- media/libstagefright/include/OMX.h | 4 +- media/libstagefright/include/OMXNodeInstance.h | 10 +- media/libstagefright/omx/OMX.cpp | 8 +- media/libstagefright/omx/OMXNodeInstance.cpp | 184 ++++++++++++++++++++++--- 7 files changed, 196 insertions(+), 41 deletions(-) diff --git a/include/media/IOMX.h b/include/media/IOMX.h index 27ad694..83a177a 100644 --- a/include/media/IOMX.h +++ b/include/media/IOMX.h @@ -107,7 +107,7 @@ public: // Use |params| as an OMX buffer, but limit the size of the OMX buffer to |allottedSize|. virtual status_t useBuffer( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) = 0; + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess = OMX_FALSE) = 0; virtual status_t useGraphicBuffer( node_id node, OMX_U32 port_index, @@ -149,7 +149,7 @@ public: // may be larger. virtual status_t allocateBufferWithBackup( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) = 0; + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess = OMX_FALSE) = 0; virtual status_t freeBuffer( node_id node, OMX_U32 port_index, buffer_id buffer) = 0; diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index b082fe4..c28eac8 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -248,7 +248,7 @@ public: virtual status_t useBuffer( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) { + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL /* crossProcess */) { Parcel data, reply; data.writeInterfaceToken(IOMX::getInterfaceDescriptor()); data.writeInt32((int32_t)node); @@ -481,7 +481,7 @@ public: virtual status_t allocateBufferWithBackup( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) { + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL /* crossProcess */) { Parcel data, reply; data.writeInterfaceToken(IOMX::getInterfaceDescriptor()); data.writeInt32((int32_t)node); @@ -834,7 +834,8 @@ status_t BnOMX::onTransact( OMX_U32 allottedSize = data.readInt32(); buffer_id buffer; - status_t err = useBuffer(node, port_index, params, &buffer, allottedSize); + status_t err = useBuffer( + node, port_index, params, &buffer, allottedSize, OMX_TRUE /* crossProcess */); reply->writeInt32(err); if (err == OK) { @@ -969,7 +970,10 @@ status_t BnOMX::onTransact( OMX_BOOL enable = (OMX_BOOL)data.readInt32(); MetadataBufferType type = kMetadataBufferTypeInvalid; - status_t err = storeMetaDataInBuffers(node, port_index, enable, &type); + status_t err = + // only control output metadata via Binder + port_index != 1 /* kOutputPortIndex */ ? BAD_VALUE : + storeMetaDataInBuffers(node, port_index, enable, &type); reply->writeInt32(type); reply->writeInt32(err); @@ -1054,7 +1058,7 @@ status_t BnOMX::onTransact( buffer_id buffer; status_t err = allocateBufferWithBackup( - node, port_index, params, &buffer, allottedSize); + node, port_index, params, &buffer, allottedSize, OMX_TRUE /* crossProcess */); reply->writeInt32(err); diff --git a/media/libstagefright/OMXClient.cpp b/media/libstagefright/OMXClient.cpp index e69890d..d252cb6 100644 --- a/media/libstagefright/OMXClient.cpp +++ b/media/libstagefright/OMXClient.cpp @@ -90,7 +90,7 @@ struct MuxOMX : public IOMX { virtual status_t useBuffer( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize); + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess); virtual status_t useGraphicBuffer( node_id node, OMX_U32 port_index, @@ -120,7 +120,7 @@ struct MuxOMX : public IOMX { virtual status_t allocateBufferWithBackup( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize); + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess); virtual status_t freeBuffer( node_id node, OMX_U32 port_index, buffer_id buffer); @@ -322,8 +322,9 @@ status_t MuxOMX::getGraphicBufferUsage( status_t MuxOMX::useBuffer( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) { - return getOMX(node)->useBuffer(node, port_index, params, buffer, allottedSize); + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL /* crossProcess */) { + return getOMX(node)->useBuffer( + node, port_index, params, buffer, allottedSize, OMX_FALSE /* crossProcess */); } status_t MuxOMX::useGraphicBuffer( @@ -375,9 +376,9 @@ status_t MuxOMX::allocateBuffer( status_t MuxOMX::allocateBufferWithBackup( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) { + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL /* crossProcess */) { return getOMX(node)->allocateBufferWithBackup( - node, port_index, params, buffer, allottedSize); + node, port_index, params, buffer, allottedSize, OMX_FALSE /* crossProcess */); } status_t MuxOMX::freeBuffer( diff --git a/media/libstagefright/include/OMX.h b/media/libstagefright/include/OMX.h index e7c4f6d..b20b2ea 100644 --- a/media/libstagefright/include/OMX.h +++ b/media/libstagefright/include/OMX.h @@ -81,7 +81,7 @@ public: virtual status_t useBuffer( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize); + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess); virtual status_t useGraphicBuffer( node_id node, OMX_U32 port_index, @@ -113,7 +113,7 @@ public: virtual status_t allocateBufferWithBackup( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize); + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess); virtual status_t freeBuffer( node_id node, OMX_U32 port_index, buffer_id buffer); diff --git a/media/libstagefright/include/OMXNodeInstance.h b/media/libstagefright/include/OMXNodeInstance.h index babf5b7..bd33ab7 100644 --- a/media/libstagefright/include/OMXNodeInstance.h +++ b/media/libstagefright/include/OMXNodeInstance.h @@ -21,6 +21,7 @@ #include "OMX.h" #include +#include #include namespace android { @@ -71,7 +72,7 @@ struct OMXNodeInstance { status_t useBuffer( OMX_U32 portIndex, const sp ¶ms, - OMX::buffer_id *buffer, OMX_U32 allottedSize); + OMX::buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess); status_t useGraphicBuffer( OMX_U32 portIndex, const sp &graphicBuffer, @@ -101,7 +102,7 @@ struct OMXNodeInstance { status_t allocateBufferWithBackup( OMX_U32 portIndex, const sp ¶ms, - OMX::buffer_id *buffer, OMX_U32 allottedSize); + OMX::buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess); status_t freeBuffer(OMX_U32 portIndex, OMX::buffer_id buffer); @@ -146,6 +147,9 @@ private: OMX_HANDLETYPE mHandle; sp mObserver; bool mDying; + bool mSailed; // configuration is set (no more meta-mode changes) + bool mQueriedProhibitedExtensions; + SortedVector mProhibitedExtensions; bool mIsSecure; // Lock only covers mGraphicBufferSource. We can't always use mLock @@ -191,6 +195,8 @@ private: OMX::buffer_id findBufferID(OMX_BUFFERHEADERTYPE *bufferHeader); void invalidateBufferID(OMX::buffer_id buffer); + bool isProhibitedIndex_l(OMX_INDEXTYPE index); + status_t useGraphicBuffer2_l( OMX_U32 portIndex, const sp &graphicBuffer, OMX::buffer_id *buffer); diff --git a/media/libstagefright/omx/OMX.cpp b/media/libstagefright/omx/OMX.cpp index 56b6055..5898b4e 100644 --- a/media/libstagefright/omx/OMX.cpp +++ b/media/libstagefright/omx/OMX.cpp @@ -368,9 +368,9 @@ status_t OMX::configureVideoTunnelMode( status_t OMX::useBuffer( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) { + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess) { return findInstance(node)->useBuffer( - port_index, params, buffer, allottedSize); + port_index, params, buffer, allottedSize, crossProcess); } status_t OMX::useGraphicBuffer( @@ -421,9 +421,9 @@ status_t OMX::allocateBuffer( status_t OMX::allocateBufferWithBackup( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) { + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess) { return findInstance(node)->allocateBufferWithBackup( - port_index, params, buffer, allottedSize); + port_index, params, buffer, allottedSize, crossProcess); } status_t OMX::freeBuffer(node_id node, OMX_U32 port_index, buffer_id buffer) { diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp index 7f534b5..a0dc2ec 100644 --- a/media/libstagefright/omx/OMXNodeInstance.cpp +++ b/media/libstagefright/omx/OMXNodeInstance.cpp @@ -101,26 +101,34 @@ static void InitOMXParams(T *params) { namespace android { struct BufferMeta { - BufferMeta(const sp &mem, OMX_U32 portIndex, bool is_backup = false) + BufferMeta( + const sp &mem, OMX_U32 portIndex, bool copyToOmx, + bool copyFromOmx, OMX_U8 *backup) : mMem(mem), - mIsBackup(is_backup), - mPortIndex(portIndex) { + mCopyFromOmx(copyFromOmx), + mCopyToOmx(copyToOmx), + mPortIndex(portIndex), + mBackup(backup) { } BufferMeta(size_t size, OMX_U32 portIndex) : mSize(size), - mIsBackup(false), - mPortIndex(portIndex) { + mCopyFromOmx(false), + mCopyToOmx(false), + mPortIndex(portIndex), + mBackup(NULL) { } BufferMeta(const sp &graphicBuffer, OMX_U32 portIndex) : mGraphicBuffer(graphicBuffer), - mIsBackup(false), - mPortIndex(portIndex) { + mCopyFromOmx(false), + mCopyToOmx(false), + mPortIndex(portIndex), + mBackup(NULL) { } void CopyFromOMX(const OMX_BUFFERHEADERTYPE *header) { - if (!mIsBackup) { + if (!mCopyFromOmx) { return; } @@ -131,7 +139,7 @@ struct BufferMeta { } void CopyToOMX(const OMX_BUFFERHEADERTYPE *header) { - if (!mIsBackup) { + if (!mCopyToOmx) { return; } @@ -167,12 +175,18 @@ struct BufferMeta { return mPortIndex; } + ~BufferMeta() { + delete[] mBackup; + } + private: sp mGraphicBuffer; sp mMem; size_t mSize; - bool mIsBackup; + bool mCopyFromOmx; + bool mCopyToOmx; OMX_U32 mPortIndex; + OMX_U8 *mBackup; BufferMeta(const BufferMeta &); BufferMeta &operator=(const BufferMeta &); @@ -199,6 +213,8 @@ OMXNodeInstance::OMXNodeInstance( mHandle(NULL), mObserver(observer), mDying(false), + mSailed(false), + mQueriedProhibitedExtensions(false), mBufferIDCount(0) { mName = ADebug::GetDebugName(name); @@ -370,7 +386,12 @@ status_t OMXNodeInstance::freeNode(OMXMaster *master) { status_t OMXNodeInstance::sendCommand( OMX_COMMANDTYPE cmd, OMX_S32 param) { - const sp& bufferSource(getGraphicBufferSource()); + if (cmd == OMX_CommandStateSet) { + // We do not support returning from unloaded state, so there are no configurations past + // first StateSet command. + mSailed = true; + } + const sp bufferSource(getGraphicBufferSource()); if (bufferSource != NULL && cmd == OMX_CommandStateSet) { if (param == OMX_StateIdle) { // Initiating transition from Executing -> Idle @@ -403,10 +424,57 @@ status_t OMXNodeInstance::sendCommand( return StatusFromOMXError(err); } +bool OMXNodeInstance::isProhibitedIndex_l(OMX_INDEXTYPE index) { + // these extensions can only be used from OMXNodeInstance, not by clients directly. + static const char *restricted_extensions[] = { + "OMX.google.android.index.storeMetaDataInBuffers", + "OMX.google.android.index.storeANWBufferInMetadata", + "OMX.google.android.index.prepareForAdaptivePlayback", + "OMX.google.android.index.configureVideoTunnelMode", + "OMX.google.android.index.useAndroidNativeBuffer2", + "OMX.google.android.index.useAndroidNativeBuffer", + "OMX.google.android.index.enableAndroidNativeBuffers", + "OMX.google.android.index.allocateNativeHandle", + "OMX.google.android.index.getAndroidNativeBufferUsage", + }; + + if ((index > OMX_IndexComponentStartUnused && index <= OMX_IndexParamStandardComponentRole) + || (index > OMX_IndexPortStartUnused && index <= OMX_IndexParamCompBufferSupplier) + || (index > OMX_IndexAudioStartUnused && index <= OMX_IndexConfigAudioChannelVolume) + || (index > OMX_IndexVideoStartUnused && index <= OMX_IndexConfigVideoNalSize) + || (index > OMX_IndexCommonStartUnused + && index <= OMX_IndexConfigCommonTransitionEffect) + || (index > (OMX_INDEXTYPE)OMX_IndexExtAudioStartUnused + && index <= (OMX_INDEXTYPE)OMX_IndexParamAudioAndroidEac3) + || (index > (OMX_INDEXTYPE)OMX_IndexExtVideoStartUnused + && index <= (OMX_INDEXTYPE)OMX_IndexParamSliceSegments) + || (index > (OMX_INDEXTYPE)OMX_IndexExtOtherStartUnused + && index <= (OMX_INDEXTYPE)OMX_IndexParamConsumerUsageBits)) { + return false; + } + + if (!mQueriedProhibitedExtensions) { + for (size_t i = 0; i < NELEM(restricted_extensions); ++i) { + OMX_INDEXTYPE ext; + if (OMX_GetExtensionIndex(mHandle, (OMX_STRING)restricted_extensions[i], &ext) == OMX_ErrorNone) { + mProhibitedExtensions.add(ext); + } + } + mQueriedProhibitedExtensions = true; + } + + return mProhibitedExtensions.indexOf(index) >= 0; +} + status_t OMXNodeInstance::getParameter( OMX_INDEXTYPE index, void *params, size_t /* size */) { Mutex::Autolock autoLock(mLock); + if (isProhibitedIndex_l(index)) { + android_errorWriteLog(0x534e4554, "29422020"); + return BAD_INDEX; + } + OMX_ERRORTYPE err = OMX_GetParameter(mHandle, index, params); OMX_INDEXEXTTYPE extIndex = (OMX_INDEXEXTTYPE)index; // some errors are expected for getParameter @@ -422,6 +490,11 @@ status_t OMXNodeInstance::setParameter( OMX_INDEXEXTTYPE extIndex = (OMX_INDEXEXTTYPE)index; CLOG_CONFIG(setParameter, "%s(%#x), %zu@%p)", asString(extIndex), index, size, params); + if (isProhibitedIndex_l(index)) { + android_errorWriteLog(0x534e4554, "29422020"); + return BAD_INDEX; + } + OMX_ERRORTYPE err = OMX_SetParameter( mHandle, index, const_cast(params)); CLOG_IF_ERROR(setParameter, err, "%s(%#x)", asString(extIndex), index); @@ -432,6 +505,11 @@ status_t OMXNodeInstance::getConfig( OMX_INDEXTYPE index, void *params, size_t /* size */) { Mutex::Autolock autoLock(mLock); + if (isProhibitedIndex_l(index)) { + android_errorWriteLog(0x534e4554, "29422020"); + return BAD_INDEX; + } + OMX_ERRORTYPE err = OMX_GetConfig(mHandle, index, params); OMX_INDEXEXTTYPE extIndex = (OMX_INDEXEXTTYPE)index; // some errors are expected for getConfig @@ -447,6 +525,11 @@ status_t OMXNodeInstance::setConfig( OMX_INDEXEXTTYPE extIndex = (OMX_INDEXEXTTYPE)index; CLOG_CONFIG(setConfig, "%s(%#x), %zu@%p)", asString(extIndex), index, size, params); + if (isProhibitedIndex_l(index)) { + android_errorWriteLog(0x534e4554, "29422020"); + return BAD_INDEX; + } + OMX_ERRORTYPE err = OMX_SetConfig( mHandle, index, const_cast(params)); CLOG_IF_ERROR(setConfig, err, "%s(%#x)", asString(extIndex), index); @@ -526,6 +609,10 @@ status_t OMXNodeInstance::storeMetaDataInBuffers( status_t OMXNodeInstance::storeMetaDataInBuffers_l( OMX_U32 portIndex, OMX_BOOL enable, MetadataBufferType *type) { + if (mSailed) { + android_errorWriteLog(0x534e4554, "29422020"); + return INVALID_OPERATION; + } if (portIndex != kPortIndexInput && portIndex != kPortIndexOutput) { android_errorWriteLog(0x534e4554, "26324358"); return BAD_VALUE; @@ -593,6 +680,10 @@ status_t OMXNodeInstance::prepareForAdaptivePlayback( OMX_U32 portIndex, OMX_BOOL enable, OMX_U32 maxFrameWidth, OMX_U32 maxFrameHeight) { Mutex::Autolock autolock(mLock); + if (mSailed) { + android_errorWriteLog(0x534e4554, "29422020"); + return INVALID_OPERATION; + } CLOG_CONFIG(prepareForAdaptivePlayback, "%s:%u en=%d max=%ux%u", portString(portIndex), portIndex, enable, maxFrameWidth, maxFrameHeight); @@ -623,6 +714,10 @@ status_t OMXNodeInstance::configureVideoTunnelMode( OMX_U32 portIndex, OMX_BOOL tunneled, OMX_U32 audioHwSync, native_handle_t **sidebandHandle) { Mutex::Autolock autolock(mLock); + if (mSailed) { + android_errorWriteLog(0x534e4554, "29422020"); + return INVALID_OPERATION; + } CLOG_CONFIG(configureVideoTunnelMode, "%s:%u tun=%d sync=%u", portString(portIndex), portIndex, tunneled, audioHwSync); @@ -663,23 +758,48 @@ status_t OMXNodeInstance::configureVideoTunnelMode( status_t OMXNodeInstance::useBuffer( OMX_U32 portIndex, const sp ¶ms, - OMX::buffer_id *buffer, OMX_U32 allottedSize) { + OMX::buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess) { Mutex::Autolock autoLock(mLock); - if (allottedSize > params->size()) { + if (allottedSize > params->size() || portIndex >= NELEM(mNumPortBuffers)) { return BAD_VALUE; } - BufferMeta *buffer_meta = new BufferMeta(params, portIndex); + // metadata buffers are not connected cross process + BufferMeta *buffer_meta; + bool isMeta = mMetadataType[portIndex] != kMetadataBufferTypeInvalid; + bool useBackup = crossProcess && isMeta; // use a backup buffer instead of the actual buffer + OMX_U8 *data = static_cast(params->pointer()); + // allocate backup buffer + if (useBackup) { + data = new (std::nothrow) OMX_U8[allottedSize]; + if (data == NULL) { + return NO_MEMORY; + } + memset(data, 0, allottedSize); + + // if we are not connecting the buffers, the sizes must match + if (allottedSize != params->size()) { + CLOG_ERROR(useBuffer, BAD_VALUE, SIMPLE_BUFFER(portIndex, (size_t)allottedSize, data)); + delete[] data; + return BAD_VALUE; + } + + buffer_meta = new BufferMeta( + params, portIndex, false /* copyToOmx */, false /* copyFromOmx */, data); + } else { + buffer_meta = new BufferMeta( + params, portIndex, false /* copyFromOmx */, false /* copyToOmx */, NULL); + } OMX_BUFFERHEADERTYPE *header; OMX_ERRORTYPE err = OMX_UseBuffer( mHandle, &header, portIndex, buffer_meta, - allottedSize, static_cast(params->pointer())); + allottedSize, data); if (err != OMX_ErrorNone) { CLOG_ERROR(useBuffer, err, SIMPLE_BUFFER( - portIndex, (size_t)allottedSize, params->pointer())); + portIndex, (size_t)allottedSize, data)); delete buffer_meta; buffer_meta = NULL; @@ -864,7 +984,16 @@ status_t OMXNodeInstance::createGraphicBufferSource( OMX_U32 portIndex, sp bufferConsumer, MetadataBufferType *type) { status_t err; - const sp& surfaceCheck = getGraphicBufferSource(); + // only allow graphic source on input port, when there are no allocated buffers yet + if (portIndex != kPortIndexInput) { + android_errorWriteLog(0x534e4554, "29422020"); + return BAD_VALUE; + } else if (mNumPortBuffers[portIndex] > 0) { + android_errorWriteLog(0x534e4554, "29422020"); + return INVALID_OPERATION; + } + + const sp surfaceCheck = getGraphicBufferSource(); if (surfaceCheck != NULL) { if (portIndex < NELEM(mMetadataType) && type != NULL) { *type = mMetadataType[portIndex]; @@ -1020,13 +1149,21 @@ status_t OMXNodeInstance::allocateBuffer( status_t OMXNodeInstance::allocateBufferWithBackup( OMX_U32 portIndex, const sp ¶ms, - OMX::buffer_id *buffer, OMX_U32 allottedSize) { + OMX::buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess) { Mutex::Autolock autoLock(mLock); - if (allottedSize > params->size()) { + if (allottedSize > params->size() || portIndex >= NELEM(mNumPortBuffers)) { return BAD_VALUE; } - BufferMeta *buffer_meta = new BufferMeta(params, portIndex, true); + // metadata buffers are not connected cross process + bool isMeta = mMetadataType[portIndex] != kMetadataBufferTypeInvalid; + bool copy = !(crossProcess && isMeta); + + BufferMeta *buffer_meta = new BufferMeta( + params, portIndex, + (portIndex == kPortIndexInput) && copy /* copyToOmx */, + (portIndex == kPortIndexOutput) && copy /* copyFromOmx */, + NULL /* data */); OMX_BUFFERHEADERTYPE *header; @@ -1044,6 +1181,7 @@ status_t OMXNodeInstance::allocateBufferWithBackup( } CHECK_EQ(header->pAppPrivate, buffer_meta); + memset(header->pBuffer, 0, header->nAllocLen); *buffer = makeBufferID(header); @@ -1122,6 +1260,12 @@ status_t OMXNodeInstance::emptyBuffer( OMX_U32 flags, OMX_TICKS timestamp, int fenceFd) { Mutex::Autolock autoLock(mLock); + // no emptybuffer if using input surface + if (getGraphicBufferSource() != NULL) { + android_errorWriteLog(0x534e4554, "29422020"); + return INVALID_OPERATION; + } + OMX_BUFFERHEADERTYPE *header = findBufferHeader(buffer, kPortIndexInput); if (header == NULL) { return BAD_VALUE; -- cgit v1.1 From 23ffe421812180b20043b8a108a72b2e95ef1f0c Mon Sep 17 00:00:00 2001 From: rago Date: Mon, 22 Aug 2016 17:20:26 -0700 Subject: Fix potential overflow in Visualizer effect Bug: 30229821 Change-Id: Idd3c1563dc9d3261e6e168e945005bf133ab2cdb (cherry picked from commit 099ab280775946e7c36c73fde47f2ee5a2579f53) (cherry picked from commit 46dc714d523a41a4f886eecbe5b9947a4c900510) --- media/libeffects/visualizer/EffectVisualizer.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/media/libeffects/visualizer/EffectVisualizer.cpp b/media/libeffects/visualizer/EffectVisualizer.cpp index 0c310c5..91f9fc7 100644 --- a/media/libeffects/visualizer/EffectVisualizer.cpp +++ b/media/libeffects/visualizer/EffectVisualizer.cpp @@ -25,6 +25,7 @@ #include #include #include +#include extern "C" { @@ -599,6 +600,14 @@ int Visualizer_command(effect_handle_t self, uint32_t cmdCode, uint32_t cmdSize, } break; case VISUALIZER_CMD_MEASURE: { + if (pReplyData == NULL || replySize == NULL || + *replySize < (sizeof(int32_t) * MEASUREMENT_COUNT)) { + ALOGV("VISUALIZER_CMD_MEASURE() error *replySize %" PRIu32 + " < (sizeof(int32_t) * MEASUREMENT_COUNT) %" PRIu32, *replySize, + sizeof(int32_t) * MEASUREMENT_COUNT); + android_errorWriteLog(0x534e4554, "30229821"); + return -EINVAL; + } uint16_t peakU16 = 0; float sumRmsSquared = 0.0f; uint8_t nbValidMeasurements = 0; -- cgit v1.1 From ace612c5f838944d1b88be18e317709e640becc2 Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Fri, 2 Sep 2016 11:36:46 -0700 Subject: Fix stack content leak vulnerability in mediaserver bug: 30875060 Change-Id: I03f4d08b7b31ac5b507cfc9e65e5607c73972d95 (cherry picked from commit 9a6861cbd3bb0e1b8fe4c105795256ee032f9664) --- media/libmedia/IDrm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/media/libmedia/IDrm.cpp b/media/libmedia/IDrm.cpp index 7c709cd..6f6530b 100644 --- a/media/libmedia/IDrm.cpp +++ b/media/libmedia/IDrm.cpp @@ -912,7 +912,7 @@ status_t BnDrm::onTransact( readVector(data, keyId); readVector(data, message); readVector(data, signature); - bool match; + bool match = false; uint32_t result = verify(sessionId, keyId, message, signature, match); reply->writeInt32(match); reply->writeInt32(result); -- cgit v1.1 From d8cf9aa813be5363fb5425b4f818a267edd5e750 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Wed, 7 Sep 2016 16:58:39 -0700 Subject: SoundTrigger: get service by value. Get strong pointer to service interface by value and not reference to prevent race conditions where the service pointer can be cleared by another thread while in use. Bug: 30907212 Change-Id: I6f02ec3fd1e6392b842b334e1cc4f9aa23916009 (cherry picked from commit 3b01b3019b1c62132792a1a94e2dc1f7cfc84f2e) --- include/soundtrigger/SoundTrigger.h | 2 +- soundtrigger/SoundTrigger.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/soundtrigger/SoundTrigger.h b/include/soundtrigger/SoundTrigger.h index bf5e1de..9a05cac 100644 --- a/include/soundtrigger/SoundTrigger.h +++ b/include/soundtrigger/SoundTrigger.h @@ -68,7 +68,7 @@ public: private: SoundTrigger(sound_trigger_module_handle_t module, const sp&); - static const sp& getSoundTriggerHwService(); + static const sp getSoundTriggerHwService(); Mutex mLock; sp mISoundTrigger; diff --git a/soundtrigger/SoundTrigger.cpp b/soundtrigger/SoundTrigger.cpp index 2138cb7..e959b83 100644 --- a/soundtrigger/SoundTrigger.cpp +++ b/soundtrigger/SoundTrigger.cpp @@ -55,7 +55,7 @@ namespace { sp gDeathNotifier; }; // namespace anonymous -const sp& SoundTrigger::getSoundTriggerHwService() +const sp SoundTrigger::getSoundTriggerHwService() { Mutex::Autolock _l(gLock); if (gSoundTriggerHwService.get() == 0) { @@ -84,7 +84,7 @@ status_t SoundTrigger::listModules(struct sound_trigger_module_descriptor *modul uint32_t *numModules) { ALOGV("listModules()"); - const sp& service = getSoundTriggerHwService(); + const sp service = getSoundTriggerHwService(); if (service == 0) { return NO_INIT; } @@ -96,7 +96,7 @@ sp SoundTrigger::attach(const sound_trigger_module_handle_t module { ALOGV("attach()"); sp soundTrigger; - const sp& service = getSoundTriggerHwService(); + const sp service = getSoundTriggerHwService(); if (service == 0) { return soundTrigger; } @@ -116,7 +116,7 @@ sp SoundTrigger::attach(const sound_trigger_module_handle_t module status_t SoundTrigger::setCaptureState(bool active) { ALOGV("setCaptureState(%d)", active); - const sp& service = getSoundTriggerHwService(); + const sp service = getSoundTriggerHwService(); if (service == 0) { return NO_INIT; } -- cgit v1.1 From 128035617a8cafd4ea717ceb8d6fe56564b07262 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Wed, 7 Sep 2016 16:58:39 -0700 Subject: Radio: get service by value. Get strong pointer to service interface by value and not reference to prevent race conditions where the service pointer can be cleared by another thread while in use. Bug: 30907212 Change-Id: Iae838b3b672562c1d0cd63968399a6bfdda7f5ab (cherry picked from commit 28a0e9ec74e5192aacffb279c80619ef284062b0) --- include/radio/Radio.h | 2 +- radio/Radio.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/radio/Radio.h b/include/radio/Radio.h index 302bf16..a4dfdd1 100644 --- a/include/radio/Radio.h +++ b/include/radio/Radio.h @@ -75,7 +75,7 @@ public: private: Radio(radio_handle_t handle, const sp&); - static const sp& getRadioService(); + static const sp getRadioService(); Mutex mLock; sp mIRadio; diff --git a/radio/Radio.cpp b/radio/Radio.cpp index e3554c2..3c04fb0 100644 --- a/radio/Radio.cpp +++ b/radio/Radio.cpp @@ -55,7 +55,7 @@ namespace { sp gDeathNotifier; }; // namespace anonymous -const sp& Radio::getRadioService() +const sp Radio::getRadioService() { Mutex::Autolock _l(gLock); if (gRadioService.get() == 0) { @@ -84,7 +84,7 @@ status_t Radio::listModules(struct radio_properties *properties, uint32_t *numModules) { ALOGV("listModules()"); - const sp& service = getRadioService(); + const sp service = getRadioService(); if (service == 0) { return NO_INIT; } @@ -98,7 +98,7 @@ sp Radio::attach(radio_handle_t handle, { ALOGV("attach()"); sp radio; - const sp& service = getRadioService(); + const sp service = getRadioService(); if (service == 0) { return radio; } -- cgit v1.1 From c2dd82bfd6d7aea5b6760efc0712ae11d7a52e6b Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Tue, 20 Sep 2016 13:36:40 -0700 Subject: Check mprotect result mprotect can theoretically fail, which could then let one exploit a vulnerable codec if one exists on the device. Bug: 31350239 Change-Id: I7b99c190619f0fb2eb93119596e6da0d2deb8ba5 (cherry picked from commit 866c800c0624bb13eee44973cc8a2ecd0012de6e) --- media/libmedia/IOMX.cpp | 52 ++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index c28eac8..365d9ac 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -733,31 +733,35 @@ status_t BnOMX::onTransact( // mark the last page as inaccessible, to avoid exploitation // of codecs that access past the end of the allocation because // they didn't check the size - mprotect((char*)params + allocSize - pageSize, pageSize, PROT_NONE); - switch (code) { - case GET_PARAMETER: - err = getParameter(node, index, params, size); - break; - case SET_PARAMETER: - err = setParameter(node, index, params, size); - break; - case GET_CONFIG: - err = getConfig(node, index, params, size); - break; - case SET_CONFIG: - err = setConfig(node, index, params, size); - break; - case SET_INTERNAL_OPTION: - { - InternalOptionType type = - (InternalOptionType)data.readInt32(); - - err = setInternalOption(node, index, type, params, size); - break; + if (mprotect((char*)params + allocSize - pageSize, pageSize, + PROT_NONE) != 0) { + ALOGE("mprotect failed: %s", strerror(errno)); + } else { + switch (code) { + case GET_PARAMETER: + err = getParameter(node, index, params, size); + break; + case SET_PARAMETER: + err = setParameter(node, index, params, size); + break; + case GET_CONFIG: + err = getConfig(node, index, params, size); + break; + case SET_CONFIG: + err = setConfig(node, index, params, size); + break; + case SET_INTERNAL_OPTION: + { + InternalOptionType type = + (InternalOptionType)data.readInt32(); + + err = setInternalOption(node, index, type, params, size); + break; + } + + default: + TRESPASS(); } - - default: - TRESPASS(); } } } -- cgit v1.1 From 2af81c25f462ba958507bfe6ba43200e3c2e2a0e Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Tue, 20 Sep 2016 17:37:55 -0700 Subject: SampleIterator: clear members on seekTo error Bug: 31091777 Change-Id: Iddf99d0011961d0fd3d755e57db4365b6a6a1193 (cherry picked from commit 03237ce0f9584c98ccda76c2474a4ae84c763f5b) --- media/libstagefright/SampleIterator.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/media/libstagefright/SampleIterator.cpp b/media/libstagefright/SampleIterator.cpp index c5f359e..335bd5d 100644 --- a/media/libstagefright/SampleIterator.cpp +++ b/media/libstagefright/SampleIterator.cpp @@ -94,8 +94,6 @@ status_t SampleIterator::seekTo(uint32_t sampleIndex) { + mFirstChunk; if (!mInitialized || chunk != mCurrentChunkIndex) { - mCurrentChunkIndex = chunk; - status_t err; if ((err = getChunkOffset(chunk, &mCurrentChunkOffset)) != OK) { ALOGE("getChunkOffset return error"); @@ -106,18 +104,21 @@ status_t SampleIterator::seekTo(uint32_t sampleIndex) { uint32_t firstChunkSampleIndex = mFirstChunkSampleIndex - + mSamplesPerChunk * (mCurrentChunkIndex - mFirstChunk); + + mSamplesPerChunk * (chunk - mFirstChunk); for (uint32_t i = 0; i < mSamplesPerChunk; ++i) { size_t sampleSize; if ((err = getSampleSizeDirect( firstChunkSampleIndex + i, &sampleSize)) != OK) { ALOGE("getSampleSizeDirect return error"); + mCurrentChunkSampleSizes.clear(); return err; } mCurrentChunkSampleSizes.push(sampleSize); } + + mCurrentChunkIndex = chunk; } uint32_t chunkRelativeSampleIndex = -- cgit v1.1 From b5203aba00dc60bee526d78e5851f0a34c4b5bd7 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Mon, 19 Sep 2016 16:22:56 -0700 Subject: Limit mp4 atom size to something reasonable Bug: 28615448 Change-Id: I5916f6839b4a9bbee4388a106e7373bcd4154f5a (cherry picked from commit cb898dca47ac03738db91ddc371207435d2a1526) --- media/libstagefright/MPEG4Extractor.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index 4c10cc9..9e7f298 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -50,6 +50,12 @@ namespace android { +enum { + // maximum size of an atom. Some atoms can be bigger according to the spec, + // but we only allow up to this size. + kMaxAtomSize = 64 * 1024 * 1024, +}; + class MPEG4Source : public MediaSource { public: // Caller retains ownership of both "dataSource" and "sampleTable". @@ -836,6 +842,13 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { PathAdder autoAdder(&mPath, chunk_type); off64_t chunk_data_size = *offset + chunk_size - data_offset; + if (chunk_type != FOURCC('m', 'd', 'a', 't') && chunk_data_size > kMaxAtomSize) { + char errMsg[100]; + sprintf(errMsg, "%s atom has size %" PRId64, chunk, chunk_data_size); + ALOGE("%s (b/28615448)", errMsg); + android_errorWriteWithInfoLog(0x534e4554, "28615448", -1, errMsg, strlen(errMsg)); + return ERROR_MALFORMED; + } if (chunk_type != FOURCC('c', 'p', 'r', 't') && chunk_type != FOURCC('c', 'o', 'v', 'r') -- cgit v1.1 From b1463a7e50d22414c62cb9872ef9078f5a962923 Mon Sep 17 00:00:00 2001 From: Lajos Molnar Date: Mon, 12 Sep 2016 14:40:47 -0700 Subject: IOMX: restrict conversion of ANWB to gralloc source in emptyBuffer This is only allowed in-process (if backup and codec buffers are connected.) Bug: 29422020 Bug: 31412859 Change-Id: If48e3e0b6f1af99a459fdc3f6f03744bbf0dc375 (cherry picked from commit 087ff38490016f4a0b6a1e717ae8af781d9b750c) --- media/libstagefright/omx/OMXNodeInstance.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp index a0dc2ec..13b7cbb 100644 --- a/media/libstagefright/omx/OMXNodeInstance.cpp +++ b/media/libstagefright/omx/OMXNodeInstance.cpp @@ -167,6 +167,10 @@ struct BufferMeta { return buf; } + bool copyToOmx() const { + return mCopyToOmx; + } + void setGraphicBuffer(const sp &graphicBuffer) { mGraphicBuffer = graphicBuffer; } @@ -788,7 +792,7 @@ status_t OMXNodeInstance::useBuffer( params, portIndex, false /* copyToOmx */, false /* copyFromOmx */, data); } else { buffer_meta = new BufferMeta( - params, portIndex, false /* copyFromOmx */, false /* copyToOmx */, NULL); + params, portIndex, false /* copyToOmx */, false /* copyFromOmx */, NULL); } OMX_BUFFERHEADERTYPE *header; @@ -1277,7 +1281,8 @@ status_t OMXNodeInstance::emptyBuffer( // convert incoming ANW meta buffers if component is configured for gralloc metadata mode // ignore rangeOffset in this case - if (mMetadataType[kPortIndexInput] == kMetadataBufferTypeGrallocSource + if (buffer_meta->copyToOmx() + && mMetadataType[kPortIndexInput] == kMetadataBufferTypeGrallocSource && backup->capacity() >= sizeof(VideoNativeMetadata) && codec->capacity() >= sizeof(VideoGrallocMetadata) && ((VideoNativeMetadata *)backup->base())->eType -- cgit v1.1 From 8cbd1c5cb14e771982965a530b34282effc39b8c Mon Sep 17 00:00:00 2001 From: Lajos Molnar Date: Wed, 14 Sep 2016 10:01:37 -0700 Subject: IOMX: allow configuration after going to loaded state This was disallowed recently but we still use it as MediaCodcec.stop only goes to loaded state, and does not free component. Bug: 31450460 Change-Id: I72e092e4e55c9f23b1baee3e950d76e84a5ef28d (cherry picked from commit e03b22839d78c841ce0a1a0a1ee1960932188b0b) --- media/libstagefright/omx/OMXNodeInstance.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp index 13b7cbb..b86611f 100644 --- a/media/libstagefright/omx/OMXNodeInstance.cpp +++ b/media/libstagefright/omx/OMXNodeInstance.cpp @@ -391,8 +391,7 @@ status_t OMXNodeInstance::freeNode(OMXMaster *master) { status_t OMXNodeInstance::sendCommand( OMX_COMMANDTYPE cmd, OMX_S32 param) { if (cmd == OMX_CommandStateSet) { - // We do not support returning from unloaded state, so there are no configurations past - // first StateSet command. + // There are no configurations past first StateSet command. mSailed = true; } const sp bufferSource(getGraphicBufferSource()); @@ -1689,6 +1688,13 @@ void OMXNodeInstance::onEvent( && arg2 == OMX_StateExecuting) { bufferSource->omxExecuting(); } + + // allow configuration if we return to the loaded state + if (event == OMX_EventCmdComplete + && arg1 == OMX_CommandStateSet + && arg2 == OMX_StateLoaded) { + mSailed = false; + } } // static -- cgit v1.1 From c2a27ba1c949d1409f75bfef5cc052b7d88e4610 Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Mon, 19 Sep 2016 15:29:04 -0700 Subject: IOMX: do not clear buffer if it's allocated by component The component might depends on their buffers to be initialized in certain ways to work. Don't clear unless we're allocating it. bug: 31586647 Change-Id: Ia0a125797e414998ef0cd8ce03672f5b1e0bbf7a (cherry picked from commit ea76573aa276f51950007217a97903c4fe64f685) --- media/libstagefright/omx/OMXNodeInstance.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp index b86611f..1e76e01 100644 --- a/media/libstagefright/omx/OMXNodeInstance.cpp +++ b/media/libstagefright/omx/OMXNodeInstance.cpp @@ -1184,7 +1184,6 @@ status_t OMXNodeInstance::allocateBufferWithBackup( } CHECK_EQ(header->pAppPrivate, buffer_meta); - memset(header->pBuffer, 0, header->nAllocLen); *buffer = makeBufferID(header); -- cgit v1.1 From c13a507aa0badeb5f8482c25e2845780ba021ce7 Mon Sep 17 00:00:00 2001 From: Lajos Molnar Date: Tue, 11 Oct 2016 08:41:51 -0700 Subject: stagefright: don't fail MediaCodec.configure if clients use store-meta key Even though storing metadata is not supported in MediaCodec.configure and is only meant to be used by Stagefright recorder, don't fail configure. Bug: 31986922 Change-Id: Id9f083be6e857e7a0d8d4a74159be5b8894e28be (cherry picked from commit ae52fd383a43ac239f459078fd003ce8ac2efb55) --- media/libstagefright/ACodec.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/media/libstagefright/ACodec.cpp b/media/libstagefright/ACodec.cpp index 6399b79..cd2408b 100644 --- a/media/libstagefright/ACodec.cpp +++ b/media/libstagefright/ACodec.cpp @@ -1655,7 +1655,10 @@ status_t ACodec::configureCodec( ALOGE("[%s] storeMetaDataInBuffers (input) failed w/ err %d", mComponentName.c_str(), err); - return err; + if (mOMX->livesLocally(mNode, getpid())) { + return err; + } + ALOGI("ignoring failure to use internal MediaCodec key."); } // For this specific case we could be using camera source even if storeMetaDataInBuffers // returns Gralloc source. Pretend that we are; this will force us to use nBufferSize. -- cgit v1.1