diff options
author | Praveen Chavan <pchavan@codeaurora.org> | 2016-04-25 10:03:42 -0700 |
---|---|---|
committer | Jessica Wagantall <jwagantall@cyngn.com> | 2016-06-08 16:54:17 -0700 |
commit | b709046dff0348f95e88bc01e19b525a6abf736c (patch) | |
tree | fcd87ccb91edcb8360fc436c4974330f98dc56a6 | |
parent | d30f731cffd81b11b90ef3d0d672c380cd7959e2 (diff) | |
download | android_hardware_qcom_media-b709046dff0348f95e88bc01e19b525a6abf736c.tar.gz android_hardware_qcom_media-b709046dff0348f95e88bc01e19b525a6abf736c.tar.bz2 android_hardware_qcom_media-b709046dff0348f95e88bc01e19b525a6abf736c.zip |
mm-video-v4l2: vdec: Avoid processing ETBs/FTBs in invalid states
(per the spec) ETB/FTB should not be handled in states other than
Executing, Paused and Idle. This avoids accessing invalid buffers.
Also add a lock to protect the private-buffers from being deleted
while accessing from another thread.
Bug: 27890802
Ticket: CYNGNOS-3020
Security Vulnerability - Heap Use-After-Free and Possible LPE in
MediaServer (libOmxVdec problem #6)
CRs-Fixed: 1008882
Change-Id: Iaac2e383cd53cf9cf8042c9ed93ddc76dba3907e
-rwxr-xr-x | mm-video-v4l2/vidc/common/inc/vidc_debug.h | 14 | ||||
-rw-r--r-- | mm-video-v4l2/vidc/vdec/inc/omx_vdec.h | 1 | ||||
-rw-r--r-- | mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp | 32 |
3 files changed, 38 insertions, 9 deletions
diff --git a/mm-video-v4l2/vidc/common/inc/vidc_debug.h b/mm-video-v4l2/vidc/common/inc/vidc_debug.h index 0ce747c4..d9007f2d 100755 --- a/mm-video-v4l2/vidc/common/inc/vidc_debug.h +++ b/mm-video-v4l2/vidc/common/inc/vidc_debug.h @@ -31,6 +31,7 @@ ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #ifdef _ANDROID_ #include <cstdio> +#include <pthread.h> enum { PRIO_ERROR=0x1, @@ -75,4 +76,17 @@ extern int debug_level; } \ } \ +class auto_lock { + public: + auto_lock(pthread_mutex_t &lock) + : mLock(lock) { + pthread_mutex_lock(&mLock); + } + ~auto_lock() { + pthread_mutex_unlock(&mLock); + } + private: + pthread_mutex_t &mLock; +}; + #endif diff --git a/mm-video-v4l2/vidc/vdec/inc/omx_vdec.h b/mm-video-v4l2/vidc/vdec/inc/omx_vdec.h index 3d8ec9ed..2d02a476 100644 --- a/mm-video-v4l2/vidc/vdec/inc/omx_vdec.h +++ b/mm-video-v4l2/vidc/vdec/inc/omx_vdec.h @@ -788,6 +788,7 @@ class omx_vdec: public qc_omx_component //************************************************************* pthread_mutex_t m_lock; pthread_mutex_t c_lock; + pthread_mutex_t buf_lock; //sem to handle the minimum procesing of commands sem_t m_cmd_lock; sem_t m_safe_flush; diff --git a/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp b/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp index a9607be3..fe0f860c 100644 --- a/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp +++ b/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp @@ -690,6 +690,7 @@ omx_vdec::omx_vdec(): m_error_propogated(false), m_vendor_config.pData = NULL; pthread_mutex_init(&m_lock, NULL); pthread_mutex_init(&c_lock, NULL); + pthread_mutex_init(&buf_lock, NULL); sem_init(&m_cmd_lock,0,0); sem_init(&m_safe_flush, 0, 0); streaming[CAPTURE_PORT] = @@ -817,6 +818,7 @@ omx_vdec::~omx_vdec() close(drv_ctx.video_driver_fd); pthread_mutex_destroy(&m_lock); pthread_mutex_destroy(&c_lock); + pthread_mutex_destroy(&buf_lock); sem_destroy(&m_cmd_lock); if (perf_flag) { DEBUG_PRINT_HIGH("--> TOTAL PROCESSING TIME"); @@ -5074,6 +5076,9 @@ OMX_ERRORTYPE omx_vdec::free_input_buffer(OMX_BUFFERHEADERTYPE *bufferHdr) index = bufferHdr - m_inp_mem_ptr; DEBUG_PRINT_LOW("Free Input Buffer index = %d",index); + auto_lock l(buf_lock); + bufferHdr->pInputPortPrivate = NULL; + if (index < drv_ctx.ip_buf.actualcount && drv_ctx.ptr_inputbuffer) { DEBUG_PRINT_LOW("Free Input Buffer index = %d",index); if (drv_ctx.ptr_inputbuffer[index].pmem_fd > 0) { @@ -6022,7 +6027,9 @@ OMX_ERRORTYPE omx_vdec::empty_this_buffer(OMX_IN OMX_HANDLETYPE hComp, OMX_ERRORTYPE ret1 = OMX_ErrorNone; unsigned int nBufferIndex = drv_ctx.ip_buf.actualcount; - if (m_state == OMX_StateInvalid) { + if (m_state != OMX_StateExecuting && + m_state != OMX_StatePause && + m_state != OMX_StateIdle) { DEBUG_PRINT_ERROR("Empty this buffer in Invalid State"); return OMX_ErrorInvalidState; } @@ -6156,9 +6163,10 @@ OMX_ERRORTYPE omx_vdec::empty_this_buffer_proxy(OMX_IN OMX_HANDLETYPE hComp, return OMX_ErrorNone; } + auto_lock l(buf_lock); temp_buffer = (struct vdec_bufferpayload *)buffer->pInputPortPrivate; - if ((temp_buffer - drv_ctx.ptr_inputbuffer) > (int)drv_ctx.ip_buf.actualcount) { + if (!temp_buffer || (temp_buffer - drv_ctx.ptr_inputbuffer) > (int)drv_ctx.ip_buf.actualcount) { return OMX_ErrorBadParameter; } /* If its first frame, H264 codec and reject is true, then parse the nal @@ -6184,7 +6192,7 @@ OMX_ERRORTYPE omx_vdec::empty_this_buffer_proxy(OMX_IN OMX_HANDLETYPE hComp, /*for use buffer we need to memcpy the data*/ temp_buffer->buffer_len = buffer->nFilledLen; - if (input_use_buffer) { + if (input_use_buffer && temp_buffer->bufferaddr) { if (buffer->nFilledLen <= temp_buffer->buffer_len) { if (arbitrary_bytes) { memcpy (temp_buffer->bufferaddr, (buffer->pBuffer + buffer->nOffset),buffer->nFilledLen); @@ -6352,6 +6360,18 @@ log_input_buffers((const char *)temp_buffer->bufferaddr, temp_buffer->buffer_len OMX_ERRORTYPE omx_vdec::fill_this_buffer(OMX_IN OMX_HANDLETYPE hComp, OMX_IN OMX_BUFFERHEADERTYPE* buffer) { + if (m_state != OMX_StateExecuting && + m_state != OMX_StatePause && + m_state != OMX_StateIdle) { + DEBUG_PRINT_ERROR("FTB in Invalid State"); + return OMX_ErrorInvalidState; + } + + if (!m_out_bEnabled) { + DEBUG_PRINT_ERROR("ERROR:FTB incorrect state operation, output port is disabled."); + return OMX_ErrorIncorrectStateOperation; + } + unsigned nPortIndex = 0; if (dynamic_buf_mode) { private_handle_t *handle = NULL; @@ -6389,12 +6409,6 @@ OMX_ERRORTYPE omx_vdec::fill_this_buffer(OMX_IN OMX_HANDLETYPE hComp, buffer->nAllocLen = handle->size; } - - if (m_state == OMX_StateInvalid) { - DEBUG_PRINT_ERROR("FTB in Invalid State"); - return OMX_ErrorInvalidState; - } - if (!m_out_bEnabled) { DEBUG_PRINT_ERROR("ERROR:FTB incorrect state operation, output port is disabled."); return OMX_ErrorIncorrectStateOperation; |