From c2e66c4ee83b4264d691d8aaabb2e94744df1e25 Mon Sep 17 00:00:00 2001 From: Praveen Chavan Date: Mon, 25 Apr 2016 10:03:42 -0700 Subject: 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 Security Vulnerability - Heap Use-After-Free and Possible LPE in MediaServer (libOmxVdec problem #6) CRs-Fixed: 1008882 Change-Id: Iaac2e383cd53cf9cf8042c9ed93ddc76dba3907e --- mm-video-v4l2/vidc/common/inc/vidc_debug.h | 14 +++++++++++ mm-video-v4l2/vidc/vdec/inc/omx_vdec.h | 1 + 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 0ce747c..d9007f2 100644 --- 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 +#include 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 2df1b6e..616b8c2 100644 --- a/mm-video-v4l2/vidc/vdec/inc/omx_vdec.h +++ b/mm-video-v4l2/vidc/vdec/inc/omx_vdec.h @@ -772,6 +772,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 646f211..f490fad 100644 --- a/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp +++ b/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp @@ -685,6 +685,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] = @@ -812,6 +813,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"); @@ -5041,6 +5043,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) { @@ -5985,7 +5990,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; } @@ -6136,9 +6143,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 @@ -6164,7 +6172,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); @@ -6340,6 +6348,18 @@ if (buffer->nFlags & QOMX_VIDEO_BUFFERFLAG_EOSEQ) { 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; @@ -6376,12 +6396,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; -- cgit v1.1