From f9ed2fe6d61259e779a37d4c2d7edb33a1c1f8ba Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Wed, 16 Mar 2016 10:32:05 -0700 Subject: Add VPX output buffer size check and handle dead observers more gracefully Bug: 27597103 Change-Id: Id7acb25d5ef69b197da15ec200a9e4f9e7b03518 --- media/libstagefright/codecs/on2/dec/SoftVPX.cpp | 23 ++++++++++++++--------- media/libstagefright/omx/OMX.cpp | 7 ++++++- 2 files changed, 20 insertions(+), 10 deletions(-) (limited to 'media/libstagefright') diff --git a/media/libstagefright/codecs/on2/dec/SoftVPX.cpp b/media/libstagefright/codecs/on2/dec/SoftVPX.cpp index e161fb8..02e85a1 100644 --- a/media/libstagefright/codecs/on2/dec/SoftVPX.cpp +++ b/media/libstagefright/codecs/on2/dec/SoftVPX.cpp @@ -149,15 +149,20 @@ bool SoftVPX::outputBuffers(bool flushDecoder, bool display, bool eos, bool *por outHeader->nFlags = 0; outHeader->nFilledLen = (outputBufferWidth() * outputBufferHeight() * 3) / 2; outHeader->nTimeStamp = *(OMX_TICKS *)mImg->user_priv; - - uint8_t *dst = outHeader->pBuffer; - const uint8_t *srcY = (const uint8_t *)mImg->planes[VPX_PLANE_Y]; - const uint8_t *srcU = (const uint8_t *)mImg->planes[VPX_PLANE_U]; - const uint8_t *srcV = (const uint8_t *)mImg->planes[VPX_PLANE_V]; - size_t srcYStride = mImg->stride[VPX_PLANE_Y]; - size_t srcUStride = mImg->stride[VPX_PLANE_U]; - size_t srcVStride = mImg->stride[VPX_PLANE_V]; - copyYV12FrameToOutputBuffer(dst, srcY, srcU, srcV, srcYStride, srcUStride, srcVStride); + if (outHeader->nAllocLen >= outHeader->nFilledLen) { + uint8_t *dst = outHeader->pBuffer; + const uint8_t *srcY = (const uint8_t *)mImg->planes[VPX_PLANE_Y]; + const uint8_t *srcU = (const uint8_t *)mImg->planes[VPX_PLANE_U]; + const uint8_t *srcV = (const uint8_t *)mImg->planes[VPX_PLANE_V]; + size_t srcYStride = mImg->stride[VPX_PLANE_Y]; + size_t srcUStride = mImg->stride[VPX_PLANE_U]; + size_t srcVStride = mImg->stride[VPX_PLANE_V]; + copyYV12FrameToOutputBuffer(dst, srcY, srcU, srcV, srcYStride, srcUStride, srcVStride); + } else { + ALOGE("b/27597103, buffer too small"); + android_errorWriteLog(0x534e4554, "27597103"); + outHeader->nFilledLen = 0; + } mImg = NULL; outInfo->mOwnedByUs = false; diff --git a/media/libstagefright/omx/OMX.cpp b/media/libstagefright/omx/OMX.cpp index 7f357c9..56b6055 100644 --- a/media/libstagefright/omx/OMX.cpp +++ b/media/libstagefright/omx/OMX.cpp @@ -179,7 +179,12 @@ void OMX::binderDied(const wp &the_late_who) { Mutex::Autolock autoLock(mLock); ssize_t index = mLiveNodes.indexOfKey(the_late_who); - CHECK(index >= 0); + + if (index < 0) { + ALOGE("b/27597103, nonexistent observer on binderDied"); + android_errorWriteLog(0x534e4554, "27597103"); + return; + } instance = mLiveNodes.editValueAt(index); mLiveNodes.removeItemsAt(index); -- cgit v1.1 From 7fd96ebfc4c9da496c59d7c45e1f62be178e626d Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Sun, 20 Mar 2016 10:44:44 +0900 Subject: codecs: check OMX buffer size before use in VP8 encoder. Bug: 27569635 Change-Id: I469573f40e21dc9f4c200749d4f220e3a2d31761 --- .../codecs/on2/enc/SoftVPXEncoder.cpp | 28 +++++++++++++++++----- 1 file changed, 22 insertions(+), 6 deletions(-) (limited to 'media/libstagefright') diff --git a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp index 410f9d0..d9895f0 100644 --- a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp +++ b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp @@ -688,9 +688,10 @@ void SoftVPXEncoder::onQueueFilled(OMX_U32 /* portIndex */) { const uint8_t *source = inputBufferHeader->pBuffer + inputBufferHeader->nOffset; + size_t frameSize = mWidth * mHeight * 3 / 2; if (mInputDataIsMeta) { source = extractGraphicBuffer( - mConversionBuffer, mWidth * mHeight * 3 / 2, + mConversionBuffer, frameSize, source, inputBufferHeader->nFilledLen, mWidth, mHeight); if (source == NULL) { @@ -698,11 +699,21 @@ void SoftVPXEncoder::onQueueFilled(OMX_U32 /* portIndex */) { notify(OMX_EventError, OMX_ErrorUndefined, 0, 0); return; } - } else if (mColorFormat == OMX_COLOR_FormatYUV420SemiPlanar) { - ConvertYUV420SemiPlanarToYUV420Planar( - source, mConversionBuffer, mWidth, mHeight); + } else { + if (inputBufferHeader->nFilledLen < frameSize) { + android_errorWriteLog(0x534e4554, "27569635"); + notify(OMX_EventError, OMX_ErrorUndefined, 0, 0); + return; + } else if (inputBufferHeader->nFilledLen > frameSize) { + ALOGW("Input buffer contains too many pixels"); + } - source = mConversionBuffer; + if (mColorFormat == OMX_COLOR_FormatYUV420SemiPlanar) { + ConvertYUV420SemiPlanarToYUV420Planar( + source, mConversionBuffer, mWidth, mHeight); + + source = mConversionBuffer; + } } vpx_image_t raw_frame; vpx_img_wrap(&raw_frame, VPX_IMG_FMT_I420, mWidth, mHeight, @@ -764,9 +775,14 @@ void SoftVPXEncoder::onQueueFilled(OMX_U32 /* portIndex */) { outputBufferHeader->nTimeStamp = encoded_packet->data.frame.pts; outputBufferHeader->nFlags = 0; if (encoded_packet->data.frame.flags & VPX_FRAME_IS_KEY) - outputBufferHeader->nFlags |= OMX_BUFFERFLAG_SYNCFRAME; + outputBufferHeader->nFlags |= OMX_BUFFERFLAG_SYNCFRAME; outputBufferHeader->nOffset = 0; outputBufferHeader->nFilledLen = encoded_packet->data.frame.sz; + if (outputBufferHeader->nFilledLen > outputBufferHeader->nAllocLen) { + android_errorWriteLog(0x534e4554, "27569635"); + notify(OMX_EventError, OMX_ErrorUndefined, 0, 0); + return; + } memcpy(outputBufferHeader->pBuffer, encoded_packet->data.frame.buf, encoded_packet->data.frame.sz); -- cgit v1.1 From 44749eb4f273f0eb681d0fa013e3beef754fa687 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Thu, 17 Mar 2016 11:15:02 -0700 Subject: SoftAMR: check output buffer size to avoid overflow. Bug: 27662364 Change-Id: I7b26892c41d6f2e690e77478ab855c2fed1ff6b0 --- media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'media/libstagefright') diff --git a/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp b/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp index a9723ea..bcf6a62 100644 --- a/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp +++ b/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp @@ -312,6 +312,15 @@ void SoftAMR::onQueueFilled(OMX_U32 /* portIndex */) { int32_t numBytesRead; if (mMode == MODE_NARROW) { + if (outHeader->nAllocLen < kNumSamplesPerFrameNB * sizeof(int16_t)) { + ALOGE("b/27662364: NB expected output buffer %zu bytes vs %u", + kNumSamplesPerFrameNB * sizeof(int16_t), outHeader->nAllocLen); + android_errorWriteLog(0x534e4554, "27662364"); + notify(OMX_EventError, OMX_ErrorOverflow, 0, NULL); + mSignalledError = true; + return; + } + numBytesRead = AMRDecode(mState, (Frame_Type_3GPP)((inputPtr[0] >> 3) & 0x0f), @@ -339,6 +348,15 @@ void SoftAMR::onQueueFilled(OMX_U32 /* portIndex */) { return; } } else { + if (outHeader->nAllocLen < kNumSamplesPerFrameWB * sizeof(int16_t)) { + ALOGE("b/27662364: WB expected output buffer %zu bytes vs %u", + kNumSamplesPerFrameWB * sizeof(int16_t), outHeader->nAllocLen); + android_errorWriteLog(0x534e4554, "27662364"); + notify(OMX_EventError, OMX_ErrorOverflow, 0, NULL); + mSignalledError = true; + return; + } + int16 mode = ((inputPtr[0] >> 3) & 0x0f); if (mode >= 10 && mode <= 13) { -- cgit v1.1 From 65756b4082cd79a2d99b2ccb5b392291fd53703f Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Fri, 18 Mar 2016 18:17:14 -0700 Subject: SoftAMR: check input buffer size to avoid overflow. Bug: 27662364 Change-Id: I47380545ea7d85845e141e722b0d84f498d27145 --- media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp | 25 +++++++++++++++++++++- .../codecs/amrnb/dec/src/amrdecode.h | 1 - .../codecs/amrnb/dec/src/gsmamr_dec.h | 14 +----------- 3 files changed, 25 insertions(+), 15 deletions(-) (limited to 'media/libstagefright') diff --git a/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp b/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp index bcf6a62..77c3742 100644 --- a/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp +++ b/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp @@ -286,6 +286,13 @@ void SoftAMR::onQueueFilled(OMX_U32 /* portIndex */) { BufferInfo *inInfo = *inQueue.begin(); OMX_BUFFERHEADERTYPE *inHeader = inInfo->mHeader; + if (inHeader->nFilledLen == 0) { + inInfo->mOwnedByUs = false; + inQueue.erase(inQueue.begin()); + notifyEmptyBufferDone(inHeader); + continue; + } + BufferInfo *outInfo = *outQueue.begin(); OMX_BUFFERHEADERTYPE *outHeader = outInfo->mHeader; @@ -321,6 +328,17 @@ void SoftAMR::onQueueFilled(OMX_U32 /* portIndex */) { return; } + int16 mode = ((inputPtr[0] >> 3) & 0x0f); + // for WMF since MIME_IETF is used when calling AMRDecode. + size_t frameSize = WmfDecBytesPerFrame[mode] + 1; + + if (inHeader->nFilledLen < frameSize) { + ALOGE("b/27662364: expected %zu bytes vs %u", frameSize, inHeader->nFilledLen); + notify(OMX_EventError, OMX_ErrorStreamCorrupt, 0, NULL); + mSignalledError = true; + return; + } + numBytesRead = AMRDecode(mState, (Frame_Type_3GPP)((inputPtr[0] >> 3) & 0x0f), @@ -370,7 +388,12 @@ void SoftAMR::onQueueFilled(OMX_U32 /* portIndex */) { } size_t frameSize = getFrameSize(mode); - CHECK_GE(inHeader->nFilledLen, frameSize); + if (inHeader->nFilledLen < frameSize) { + ALOGE("b/27662364: expected %zu bytes vs %u", frameSize, inHeader->nFilledLen); + notify(OMX_EventError, OMX_ErrorStreamCorrupt, 0, NULL); + mSignalledError = true; + return; + } int16_t *outPtr = (int16_t *)outHeader->pBuffer; diff --git a/media/libstagefright/codecs/amrnb/dec/src/amrdecode.h b/media/libstagefright/codecs/amrnb/dec/src/amrdecode.h index 0988e17..f224fb6 100644 --- a/media/libstagefright/codecs/amrnb/dec/src/amrdecode.h +++ b/media/libstagefright/codecs/amrnb/dec/src/amrdecode.h @@ -104,7 +104,6 @@ terms listed above has been obtained from the copyright holder. ; INCLUDES ----------------------------------------------------------------------------*/ #include "typedef.h" -#include "mode.h" #include "frame_type_3gpp.h" /*--------------------------------------------------------------------------*/ diff --git a/media/libstagefright/codecs/amrnb/dec/src/gsmamr_dec.h b/media/libstagefright/codecs/amrnb/dec/src/gsmamr_dec.h index 8f54ee8..dc64d67 100644 --- a/media/libstagefright/codecs/amrnb/dec/src/gsmamr_dec.h +++ b/media/libstagefright/codecs/amrnb/dec/src/gsmamr_dec.h @@ -87,6 +87,7 @@ terms listed above has been obtained from the copyright holder. #include "gsm_amr_typedefs.h" #include "frame_type_3gpp.h" +#include "amrdecode.h" /*--------------------------------------------------------------------------*/ #ifdef __cplusplus @@ -136,19 +137,6 @@ extern "C" Word8 *id); /* - * AMRDecode steps into the part of the library that decodes the raw data - * speech bits for the decoding process. It returns the address offset of - * the next frame to be decoded. - */ - Word16 AMRDecode( - void *state_data, - enum Frame_Type_3GPP frame_type, - UWord8 *speech_bits_ptr, - Word16 *raw_pcm_buffer, - Word16 input_format - ); - - /* * This function resets the state memory used by the GSM AMR decoder. This * function returns zero. It will return negative one if there is an error. */ -- cgit v1.1 From daa85dac2055b22dabbb3b4e537597e6ab73a866 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Fri, 25 Mar 2016 08:26:18 -0700 Subject: Fix AMR decoder Previous change caused EOS to be ignored. Bug: 27843673 Related-to-bug: 27662364 Change-Id: Ia148a88abc861a9b393f42bc7cd63d8d3ae349bc --- media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'media/libstagefright') diff --git a/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp b/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp index 77c3742..955309a 100644 --- a/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp +++ b/media/libstagefright/codecs/amrnb/dec/SoftAMR.cpp @@ -286,13 +286,6 @@ void SoftAMR::onQueueFilled(OMX_U32 /* portIndex */) { BufferInfo *inInfo = *inQueue.begin(); OMX_BUFFERHEADERTYPE *inHeader = inInfo->mHeader; - if (inHeader->nFilledLen == 0) { - inInfo->mOwnedByUs = false; - inQueue.erase(inQueue.begin()); - notifyEmptyBufferDone(inHeader); - continue; - } - BufferInfo *outInfo = *outQueue.begin(); OMX_BUFFERHEADERTYPE *outHeader = outInfo->mHeader; @@ -310,6 +303,13 @@ void SoftAMR::onQueueFilled(OMX_U32 /* portIndex */) { return; } + if (inHeader->nFilledLen == 0) { + inInfo->mOwnedByUs = false; + inQueue.erase(inQueue.begin()); + notifyEmptyBufferDone(inHeader); + continue; + } + if (inHeader->nOffset == 0) { mAnchorTimeUs = inHeader->nTimeStamp; mNumSamplesOutput = 0; -- cgit v1.1