From 31cc990c1a8b4d2a25634a5275e85bb20ca34447 Mon Sep 17 00:00:00 2001 From: Santhosh Behara Date: Fri, 15 Sep 2017 05:28:04 -0700 Subject: [PATCH 1/5] mm-video-v4l2: venc: Avoid buffer access after free client expects buffer to be free if free_buffer is called, but if omx is in executing state free buffer call will error out. When async thread tries to copy data to client buffer which is already freed,it leads to crash. Added a bitmask to avoid copy to buffer after free. Bug: 36130225 CRs-Fixed: 2106434 Test: build & boot Test: cts-tradefed run cts-dev --module CtsMediaTestCases --compatibility:module-arg CtsMediaTestCases:include-annotation:android.platform.test.annotations.RequiresDevice Author: Uma Mehta Change-Id: Id439aac54ee64a65ea68b6431a9f5150255a6980 --- .../vidc/venc/inc/omx_video_base.h | 1 + .../vidc/venc/src/omx_video_base.cpp | 18 ++++++++++++------ .../vidc/venc/src/omx_video_encoder.cpp | 7 ++++++- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/msm8996/mm-video-v4l2/vidc/venc/inc/omx_video_base.h b/msm8996/mm-video-v4l2/vidc/venc/inc/omx_video_base.h index 33671e8b..ffbbb378 100644 --- a/msm8996/mm-video-v4l2/vidc/venc/inc/omx_video_base.h +++ b/msm8996/mm-video-v4l2/vidc/venc/inc/omx_video_base.h @@ -677,6 +677,7 @@ class omx_video: public qc_omx_component int pending_output_buffers; uint64_t m_out_bm_count; + uint64_t m_client_out_bm_count; uint64_t m_inp_bm_count; uint64_t m_flags; uint64_t m_etb_count; diff --git a/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp b/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp index 6175da08..19357cc1 100644 --- a/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp +++ b/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp @@ -295,6 +295,7 @@ omx_video::omx_video(): pending_input_buffers(0), pending_output_buffers(0), m_out_bm_count(0), + m_client_out_bm_count(0), m_inp_bm_count(0), m_flags(0), m_etb_count(0), @@ -2668,7 +2669,6 @@ OMX_ERRORTYPE omx_video::use_output_buffer( return OMX_ErrorBadParameter; } - auto_lock l(m_buf_lock); if (!m_out_mem_ptr) { output_use_buffer = true; int nBufHdrSize = 0; @@ -2819,6 +2819,7 @@ OMX_ERRORTYPE omx_video::use_output_buffer( } BITMASK_SET(&m_out_bm_count,i); + BITMASK_SET(&m_client_out_bm_count,i); } else { DEBUG_PRINT_ERROR("ERROR: All o/p Buffers have been Used, invalid use_buf call for " "index = %u", i); @@ -2856,6 +2857,8 @@ OMX_ERRORTYPE omx_video::use_buffer( DEBUG_PRINT_ERROR("ERROR: Use Buffer in Invalid State"); return OMX_ErrorInvalidState; } + + auto_lock l(m_buf_lock); if (port == PORT_INDEX_IN) { eRet = use_input_buffer(hComp,bufferHdr,port,appData,bytes,buffer); } else if (port == PORT_INDEX_OUT) { @@ -2927,7 +2930,6 @@ OMX_ERRORTYPE omx_video::free_input_buffer(OMX_BUFFERHEADERTYPE *bufferHdr) } if (index < m_sInPortDef.nBufferCountActual && m_pInput_pmem) { - auto_lock l(m_lock); if (mUseProxyColorFormat) { if (m_opq_pmem_q.m_size) { @@ -3447,7 +3449,7 @@ OMX_ERRORTYPE omx_video::allocate_buffer(OMX_IN OMX_HANDLETYPE h DEBUG_PRINT_ERROR("ERROR: Allocate Buf in Invalid State"); return OMX_ErrorInvalidState; } - + auto_lock l(m_buf_lock); // What if the client calls again. if (port == PORT_INDEX_IN) { #ifdef _ANDROID_ICS_ @@ -3518,7 +3520,12 @@ OMX_ERRORTYPE omx_video::free_buffer(OMX_IN OMX_HANDLETYPE hComp, unsigned int nPortIndex; DEBUG_PRINT_LOW("In for encoder free_buffer"); - + auto_lock l(m_buf_lock); + if (port == PORT_INDEX_OUT) { //client called freebuffer, clearing client buffer bitmask right away to avoid use after free + nPortIndex = buffer - (OMX_BUFFERHEADERTYPE*)m_out_mem_ptr; + if(BITMASK_PRESENT(&m_client_out_bm_count, nPortIndex)) + BITMASK_CLEAR(&m_client_out_bm_count,nPortIndex); + } if (m_state == OMX_StateIdle && (BITMASK_PRESENT(&m_flags ,OMX_COMPONENT_LOADING_PENDING))) { DEBUG_PRINT_LOW(" free buffer while Component in Loading pending"); @@ -3595,7 +3602,6 @@ OMX_ERRORTYPE omx_video::free_buffer(OMX_IN OMX_HANDLETYPE hComp, nPortIndex, (unsigned int)m_sOutPortDef.nBufferCountActual); if (nPortIndex < m_sOutPortDef.nBufferCountActual && BITMASK_PRESENT(&m_out_bm_count, nPortIndex)) { - auto_lock l(m_buf_lock); // Clear the bit associated with it. BITMASK_CLEAR(&m_out_bm_count,nPortIndex); m_sOutPortDef.bPopulated = OMX_FALSE; @@ -3856,7 +3862,7 @@ OMX_ERRORTYPE omx_video::empty_this_buffer_proxy(OMX_IN OMX_HANDLETYPE hComp, { DEBUG_PRINT_LOW("Heap UseBuffer case, so memcpy the data"); - auto_lock l(m_lock); + auto_lock l(m_buf_lock); pmem_data_buf = (OMX_U8 *)m_pInput_pmem[nBufIndex].buffer; if (pmem_data_buf) { memcpy (pmem_data_buf, (buffer->pBuffer + buffer->nOffset), diff --git a/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp b/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp index 9faf8c29..c89d2d05 100644 --- a/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp +++ b/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp @@ -2190,11 +2190,15 @@ OMX_ERRORTYPE omx_venc::component_deinit(OMX_IN OMX_HANDLETYPE hComp) DEBUG_PRINT_ERROR("WARNING:Rxd DeInit,OMX not in LOADED state %d",\ m_state); } + + auto_lock l(m_buf_lock); if (m_out_mem_ptr) { DEBUG_PRINT_LOW("Freeing the Output Memory"); for (i=0; i< m_sOutPortDef.nBufferCountActual; i++ ) { if (BITMASK_PRESENT(&m_out_bm_count, i)) { BITMASK_CLEAR(&m_out_bm_count, i); + if (BITMASK_PRESENT(&m_client_out_bm_count, i)) + BITMASK_CLEAR(&m_client_out_bm_count, i); free_output_buffer (&m_out_mem_ptr[i]); } @@ -2546,7 +2550,8 @@ int omx_venc::async_message_process (void *context, void* message) omxhdr->nFlags = m_sVenc_msg->buf.flags; /*Use buffer case*/ - if (omx->output_use_buffer && !omx->m_use_output_pmem && !omx->is_secure_session()) { + if (BITMASK_PRESENT(&(omx->m_client_out_bm_count), bufIndex) && + omx->output_use_buffer && !omx->m_use_output_pmem && !omx->is_secure_session()) { DEBUG_PRINT_LOW("memcpy() for o/p Heap UseBuffer"); memcpy(omxhdr->pBuffer, (m_sVenc_msg->buf.ptrbuffer), From f8c8a4eb4ee607a502fdc656e38ebe483bf2c570 Mon Sep 17 00:00:00 2001 From: Santhosh Behara Date: Mon, 3 Jul 2017 15:56:44 +0530 Subject: [PATCH 2/5] mm-video-v4l2: Protect buffer lifecycle with lock IL Client may choose to free the buffer which might be in-use by IL component. This may lead to use-after free situation. Protect buffer lifecycle with a lock to ensure that IL component operates on a buffer which exists. Fixes bug 62452543 Security Vulnerability - Heap use after free in libOmxVenc CRs-Fixed: 2062772 Test: build & boot Test: cts-tradefed run cts-dev --module CtsMediaTestCases --compatibility:module-arg CtsMediaTestCases:include-annotation:android.platform.test.annotations.RequiresDevice Author: Vikash Garodia Change-Id: I0fdb4051c94044e032c257febbe2ba1c7e4d6c7e --- .../mm-video-v4l2/vidc/venc/src/omx_video_base.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp b/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp index 19357cc1..a8993cd9 100644 --- a/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp +++ b/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp @@ -2860,6 +2860,7 @@ OMX_ERRORTYPE omx_video::use_buffer( auto_lock l(m_buf_lock); if (port == PORT_INDEX_IN) { + auto_lock l(m_lock); eRet = use_input_buffer(hComp,bufferHdr,port,appData,bytes,buffer); } else if (port == PORT_INDEX_OUT) { eRet = use_output_buffer(hComp,bufferHdr,port,appData,bytes,buffer); @@ -3050,7 +3051,9 @@ OMX_ERRORTYPE omx_video::allocate_input_meta_buffer( meta_buffer_hdr, m_inp_mem_ptr); } for (index = 0; ((index < m_sInPortDef.nBufferCountActual) && - meta_buffer_hdr[index].pBuffer); index++); + meta_buffer_hdr[index].pBuffer && + BITMASK_PRESENT(&m_inp_bm_count, index)); index++); + if (index == m_sInPortDef.nBufferCountActual) { DEBUG_PRINT_ERROR("All buffers are allocated input_meta_buffer"); return OMX_ErrorBadParameter; @@ -3452,6 +3455,7 @@ OMX_ERRORTYPE omx_video::allocate_buffer(OMX_IN OMX_HANDLETYPE h auto_lock l(m_buf_lock); // What if the client calls again. if (port == PORT_INDEX_IN) { + auto_lock l(m_lock); #ifdef _ANDROID_ICS_ if (meta_mode_enable) eRet = allocate_input_meta_buffer(hComp,bufferHdr,appData,bytes); @@ -3551,10 +3555,12 @@ OMX_ERRORTYPE omx_video::free_buffer(OMX_IN OMX_HANDLETYPE hComp, DEBUG_PRINT_LOW("free_buffer on i/p port - Port idx %u, actual cnt %u", nPortIndex, (unsigned int)m_sInPortDef.nBufferCountActual); + pthread_mutex_lock(&m_lock); if (nPortIndex < m_sInPortDef.nBufferCountActual && BITMASK_PRESENT(&m_inp_bm_count, nPortIndex)) { // Clear the bit associated with it. BITMASK_CLEAR(&m_inp_bm_count,nPortIndex); + pthread_mutex_unlock(&m_lock); free_input_buffer (buffer); m_sInPortDef.bPopulated = OMX_FALSE; @@ -3582,6 +3588,7 @@ OMX_ERRORTYPE omx_video::free_buffer(OMX_IN OMX_HANDLETYPE hComp, #endif } } else { + pthread_mutex_unlock(&m_lock); DEBUG_PRINT_ERROR("ERROR: free_buffer ,Port Index Invalid"); eRet = OMX_ErrorBadPortIndex; } @@ -3864,7 +3871,7 @@ OMX_ERRORTYPE omx_video::empty_this_buffer_proxy(OMX_IN OMX_HANDLETYPE hComp, auto_lock l(m_buf_lock); pmem_data_buf = (OMX_U8 *)m_pInput_pmem[nBufIndex].buffer; - if (pmem_data_buf) { + if (pmem_data_buf && BITMASK_PRESENT(&m_inp_bm_count, nBufIndex)) { memcpy (pmem_data_buf, (buffer->pBuffer + buffer->nOffset), buffer->nFilledLen); } From 9f1dee37d43e3af1f21a846080f5a6a86944ff01 Mon Sep 17 00:00:00 2001 From: Santhosh Behara Date: Thu, 21 Sep 2017 23:54:05 -0700 Subject: [PATCH 3/5] mm-video-v4l2: venc: Use client allocated memory if available IL client may free the buffer and calls for free buffer on IL component to free the buffer header. It may happen that the IL component may reject the free buffer due to various reasons. In such scenario, client might have already freed the memory allocated by client (such scenario will appear in use buffer mode of buffer allocation). Now accessing client buffer in such scenario may lead to use after free vulnerability. Added a flag to indicate if the client buffer is available to perform any operation on the client allocated memory. If not, restrict from doing any operation on client memory. Bug: 62452543 CRs-Fixed: 2106434 Test: build & boot Test: cts-tradefed run cts-dev --module CtsMediaTestCases --compatibility:module-arg CtsMediaTestCases:include-annotation:android.platform.test.annotations.RequiresDevice Author: Vikash Garodia Change-Id: I45e4f117e98588ee7c888ec5c1cb2424bc7e5fa3 --- msm8996/mm-video-v4l2/vidc/venc/inc/omx_video_base.h | 1 + msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp | 8 +++++++- msm8996/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/msm8996/mm-video-v4l2/vidc/venc/inc/omx_video_base.h b/msm8996/mm-video-v4l2/vidc/venc/inc/omx_video_base.h index ffbbb378..61aee806 100644 --- a/msm8996/mm-video-v4l2/vidc/venc/inc/omx_video_base.h +++ b/msm8996/mm-video-v4l2/vidc/venc/inc/omx_video_base.h @@ -678,6 +678,7 @@ class omx_video: public qc_omx_component uint64_t m_out_bm_count; uint64_t m_client_out_bm_count; + uint64_t m_client_in_bm_count; uint64_t m_inp_bm_count; uint64_t m_flags; uint64_t m_etb_count; diff --git a/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp b/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp index a8993cd9..02e9ab01 100644 --- a/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp +++ b/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp @@ -296,6 +296,7 @@ omx_video::omx_video(): pending_output_buffers(0), m_out_bm_count(0), m_client_out_bm_count(0), + m_client_in_bm_count(0), m_inp_bm_count(0), m_flags(0), m_etb_count(0), @@ -2541,6 +2542,7 @@ OMX_ERRORTYPE omx_video::use_input_buffer( *bufferHdr = (m_inp_mem_ptr + i); BITMASK_SET(&m_inp_bm_count,i); + BITMASK_SET(&m_client_in_bm_count,i); (*bufferHdr)->pBuffer = (OMX_U8 *)buffer; (*bufferHdr)->nSize = sizeof(OMX_BUFFERHEADERTYPE); @@ -3529,6 +3531,10 @@ OMX_ERRORTYPE omx_video::free_buffer(OMX_IN OMX_HANDLETYPE hComp, nPortIndex = buffer - (OMX_BUFFERHEADERTYPE*)m_out_mem_ptr; if(BITMASK_PRESENT(&m_client_out_bm_count, nPortIndex)) BITMASK_CLEAR(&m_client_out_bm_count,nPortIndex); + } else if (port == PORT_INDEX_IN) { + nPortIndex = buffer - (meta_mode_enable?meta_buffer_hdr:m_inp_mem_ptr); + if(BITMASK_PRESENT(&m_client_in_bm_count, nPortIndex)) + BITMASK_CLEAR(&m_client_in_bm_count,nPortIndex); } if (m_state == OMX_StateIdle && (BITMASK_PRESENT(&m_flags ,OMX_COMPONENT_LOADING_PENDING))) { @@ -3871,7 +3877,7 @@ OMX_ERRORTYPE omx_video::empty_this_buffer_proxy(OMX_IN OMX_HANDLETYPE hComp, auto_lock l(m_buf_lock); pmem_data_buf = (OMX_U8 *)m_pInput_pmem[nBufIndex].buffer; - if (pmem_data_buf && BITMASK_PRESENT(&m_inp_bm_count, nBufIndex)) { + if (pmem_data_buf && BITMASK_PRESENT(&m_client_in_bm_count, nBufIndex)) { memcpy (pmem_data_buf, (buffer->pBuffer + buffer->nOffset), buffer->nFilledLen); } diff --git a/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp b/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp index c89d2d05..f103a139 100644 --- a/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp +++ b/msm8996/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp @@ -2220,6 +2220,8 @@ OMX_ERRORTYPE omx_venc::component_deinit(OMX_IN OMX_HANDLETYPE hComp) for (i=0; i Date: Thu, 14 Sep 2017 16:53:45 +0530 Subject: [PATCH 4/5] mm-video-v4l2: venc: Avoid buffer access after free client expects buffer to be free if free_buffer is called, but if omx is in executing state free buffer call will error out. When async thread tries to copy data to client buffer which is already freed,it leads to crash. Added a bitmask to avoid copy to buffer after free. Bug: 36130225 CRs-Fixed: 2106434 Test: build & boot Test: cts-tradefed run cts-dev --module CtsMediaTestCases --compatibility:module-arg CtsMediaTestCases:include-annotation:android.platform.test.annotations.RequiresDevice Change-Id: I4eb44837f9b3f8f1b8b2b4c879d8c3dd470bb52f Author: Uma Mehta --- .../vidc/venc/inc/omx_video_base.h | 1 + .../vidc/venc/src/omx_video_base.cpp | 19 ++++++++++++------- .../vidc/venc/src/omx_video_encoder.cpp | 7 ++++++- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h b/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h index 13b5025a..afe31ef9 100644 --- a/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h +++ b/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h @@ -702,6 +702,7 @@ class omx_video: public qc_omx_component bool allocate_native_handle; uint64_t m_out_bm_count; + uint64_t m_client_out_bm_count; uint64_t m_inp_bm_count; uint64_t m_flags; uint64_t m_etb_count; diff --git a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp index 5d0f4458..3bcd72cb 100644 --- a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp +++ b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp @@ -289,6 +289,7 @@ omx_video::omx_video(): pending_output_buffers(0), allocate_native_handle(false), m_out_bm_count(0), + m_client_out_bm_count(0), m_inp_bm_count(0), m_flags(0), m_etb_count(0), @@ -2751,7 +2752,6 @@ OMX_ERRORTYPE omx_video::use_output_buffer( return OMX_ErrorBadParameter; } - auto_lock l(m_buf_lock); if (!m_out_mem_ptr) { output_use_buffer = true; int nBufHdrSize = 0; @@ -2902,6 +2902,7 @@ OMX_ERRORTYPE omx_video::use_output_buffer( } BITMASK_SET(&m_out_bm_count,i); + BITMASK_SET(&m_client_out_bm_count,i); } else { DEBUG_PRINT_ERROR("ERROR: All o/p Buffers have been Used, invalid use_buf call for " "index = %u", i); @@ -2939,6 +2940,8 @@ OMX_ERRORTYPE omx_video::use_buffer( DEBUG_PRINT_ERROR("ERROR: Use Buffer in Invalid State"); return OMX_ErrorInvalidState; } + + auto_lock l(m_buf_lock); if (port == PORT_INDEX_IN) { auto_lock l(m_lock); eRet = use_input_buffer(hComp,bufferHdr,port,appData,bytes,buffer); @@ -2948,7 +2951,6 @@ OMX_ERRORTYPE omx_video::use_buffer( DEBUG_PRINT_ERROR("ERROR: Invalid Port Index received %d",(int)port); eRet = OMX_ErrorBadPortIndex; } - if (eRet == OMX_ErrorNone) { if (allocate_done()) { if (BITMASK_PRESENT(&m_flags,OMX_COMPONENT_IDLE_PENDING)) { @@ -3011,7 +3013,6 @@ OMX_ERRORTYPE omx_video::free_input_buffer(OMX_BUFFERHEADERTYPE *bufferHdr) } if (index < m_sInPortDef.nBufferCountActual && m_pInput_pmem) { - auto_lock l(m_lock); if (mUseProxyColorFormat) { if (m_opq_pmem_q.m_size) { @@ -3565,7 +3566,7 @@ OMX_ERRORTYPE omx_video::allocate_buffer(OMX_IN OMX_HANDLETYPE h DEBUG_PRINT_ERROR("ERROR: Allocate Buf in Invalid State"); return OMX_ErrorInvalidState; } - + auto_lock l(m_buf_lock); // What if the client calls again. if (port == PORT_INDEX_IN) { auto_lock l(m_lock); @@ -3637,7 +3638,12 @@ OMX_ERRORTYPE omx_video::free_buffer(OMX_IN OMX_HANDLETYPE hComp, unsigned int nPortIndex; DEBUG_PRINT_LOW("In for encoder free_buffer"); - + auto_lock l(m_buf_lock); + if (port == PORT_INDEX_OUT) { //client called freebuffer, clearing client buffer bitmask right away to avoid use after free + nPortIndex = buffer - (OMX_BUFFERHEADERTYPE*)m_out_mem_ptr; + if(BITMASK_PRESENT(&m_client_out_bm_count, nPortIndex)) + BITMASK_CLEAR(&m_client_out_bm_count,nPortIndex); + } if (m_state == OMX_StateIdle && (BITMASK_PRESENT(&m_flags ,OMX_COMPONENT_LOADING_PENDING))) { DEBUG_PRINT_LOW(" free buffer while Component in Loading pending"); @@ -3717,7 +3723,6 @@ OMX_ERRORTYPE omx_video::free_buffer(OMX_IN OMX_HANDLETYPE hComp, nPortIndex, (unsigned int)m_sOutPortDef.nBufferCountActual); if (nPortIndex < m_sOutPortDef.nBufferCountActual && BITMASK_PRESENT(&m_out_bm_count, nPortIndex)) { - auto_lock l(m_buf_lock); // Clear the bit associated with it. BITMASK_CLEAR(&m_out_bm_count,nPortIndex); m_sOutPortDef.bPopulated = OMX_FALSE; @@ -3997,7 +4002,7 @@ OMX_ERRORTYPE omx_video::empty_this_buffer_proxy(OMX_IN OMX_HANDLETYPE hComp, { DEBUG_PRINT_LOW("Heap UseBuffer case, so memcpy the data"); - auto_lock l(m_lock); + auto_lock l(m_buf_lock); pmem_data_buf = (OMX_U8 *)m_pInput_pmem[nBufIndex].buffer; if (pmem_data_buf && BITMASK_PRESENT(&m_inp_bm_count, nBufIndex)) { memcpy (pmem_data_buf, (buffer->pBuffer + buffer->nOffset), diff --git a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp index b8ee0935..20213b30 100644 --- a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp +++ b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp @@ -2361,11 +2361,15 @@ OMX_ERRORTYPE omx_venc::component_deinit(OMX_IN OMX_HANDLETYPE hComp) DEBUG_PRINT_ERROR("WARNING:Rxd DeInit,OMX not in LOADED state %d",\ m_state); } + + auto_lock l(m_buf_lock); if (m_out_mem_ptr) { DEBUG_PRINT_LOW("Freeing the Output Memory"); for (i=0; i< m_sOutPortDef.nBufferCountActual; i++ ) { if (BITMASK_PRESENT(&m_out_bm_count, i)) { BITMASK_CLEAR(&m_out_bm_count, i); + if (BITMASK_PRESENT(&m_client_out_bm_count, i)) + BITMASK_CLEAR(&m_client_out_bm_count, i); free_output_buffer (&m_out_mem_ptr[i]); } @@ -2725,7 +2729,8 @@ int omx_venc::async_message_process (void *context, void* message) omxhdr->nFlags = m_sVenc_msg->buf.flags; /*Use buffer case*/ - if (omx->output_use_buffer && !omx->m_use_output_pmem && !omx->is_secure_session()) { + if (BITMASK_PRESENT(&(omx->m_client_out_bm_count), bufIndex) && + omx->output_use_buffer && !omx->m_use_output_pmem && !omx->is_secure_session()) { DEBUG_PRINT_LOW("memcpy() for o/p Heap UseBuffer"); memcpy(omxhdr->pBuffer, (m_sVenc_msg->buf.ptrbuffer), From 38641613a6eb7b761429fcdfa31218e8c63c1c56 Mon Sep 17 00:00:00 2001 From: Santhosh Behara Date: Tue, 19 Sep 2017 12:43:02 +0530 Subject: [PATCH 5/5] mm-video-v4l2: venc: Use client allocated memory if available IL client may free the buffer and calls for free buffer on IL component to free the buffer header. It may happen that the IL component may reject the free buffer due to various reasons. In such scenario, client might have already freed the memory allocated by client (such scenario will appear in use buffer mode of buffer allocation). Now accessing client buffer in such scenario may lead to use after free vulnerability. Added a flag to indicate if the client buffer is available to perform any operation on the client allocated memory. If not, restrict from doing any operation on client memory. Bug: 62452543 CRs-Fixed: 2106434 Test: build & boot Test: cts-tradefed run cts-dev --module CtsMediaTestCases --compatibility:module-arg CtsMediaTestCases:include-annotation:android.platform.test.annotations.RequiresDevice Change-Id: If24c36b9a1cca36a2728d3aec8ab589a48a9da35 Author: Vikash Garodia --- msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h | 1 + msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp | 8 +++++++- msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h b/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h index afe31ef9..26ca1f1b 100644 --- a/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h +++ b/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h @@ -703,6 +703,7 @@ class omx_video: public qc_omx_component uint64_t m_out_bm_count; uint64_t m_client_out_bm_count; + uint64_t m_client_in_bm_count; uint64_t m_inp_bm_count; uint64_t m_flags; uint64_t m_etb_count; diff --git a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp index 3bcd72cb..003b2b68 100644 --- a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp +++ b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp @@ -290,6 +290,7 @@ omx_video::omx_video(): allocate_native_handle(false), m_out_bm_count(0), m_client_out_bm_count(0), + m_client_in_bm_count(0), m_inp_bm_count(0), m_flags(0), m_etb_count(0), @@ -2624,6 +2625,7 @@ OMX_ERRORTYPE omx_video::use_input_buffer( *bufferHdr = (m_inp_mem_ptr + i); BITMASK_SET(&m_inp_bm_count,i); + BITMASK_SET(&m_client_in_bm_count,i); (*bufferHdr)->pBuffer = (OMX_U8 *)buffer; (*bufferHdr)->nSize = sizeof(OMX_BUFFERHEADERTYPE); @@ -3643,6 +3645,10 @@ OMX_ERRORTYPE omx_video::free_buffer(OMX_IN OMX_HANDLETYPE hComp, nPortIndex = buffer - (OMX_BUFFERHEADERTYPE*)m_out_mem_ptr; if(BITMASK_PRESENT(&m_client_out_bm_count, nPortIndex)) BITMASK_CLEAR(&m_client_out_bm_count,nPortIndex); + } else if (port == PORT_INDEX_IN) { + nPortIndex = buffer - (meta_mode_enable?meta_buffer_hdr:m_inp_mem_ptr); + if(BITMASK_PRESENT(&m_client_in_bm_count, nPortIndex)) + BITMASK_CLEAR(&m_client_in_bm_count,nPortIndex); } if (m_state == OMX_StateIdle && (BITMASK_PRESENT(&m_flags ,OMX_COMPONENT_LOADING_PENDING))) { @@ -4004,7 +4010,7 @@ OMX_ERRORTYPE omx_video::empty_this_buffer_proxy(OMX_IN OMX_HANDLETYPE hComp, auto_lock l(m_buf_lock); pmem_data_buf = (OMX_U8 *)m_pInput_pmem[nBufIndex].buffer; - if (pmem_data_buf && BITMASK_PRESENT(&m_inp_bm_count, nBufIndex)) { + if (pmem_data_buf && BITMASK_PRESENT(&m_client_in_bm_count, nBufIndex)) { memcpy (pmem_data_buf, (buffer->pBuffer + buffer->nOffset), buffer->nFilledLen); } diff --git a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp index 20213b30..526ebb49 100644 --- a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp +++ b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp @@ -2391,6 +2391,8 @@ OMX_ERRORTYPE omx_venc::component_deinit(OMX_IN OMX_HANDLETYPE hComp) for (i=0; i