diff options
author | Surajit Podder <spodder@codeaurora.org> | 2017-11-27 18:56:59 +0530 |
---|---|---|
committer | Gerrit - the friendly Code Review server <code-review@localhost> | 2017-11-27 05:44:52 -0800 |
commit | a5bd96ec0cd01156af6a50a1ad20b4030212dc71 (patch) | |
tree | aa147830f6e0b038931fc0971ebed2c976258164 /mm-video-v4l2 | |
parent | 754fe7f1f2b6e80b7631c12d1126211c4a20910e (diff) | |
download | media-a5bd96ec0cd01156af6a50a1ad20b4030212dc71.tar.gz |
mm-video-v4l2: venc: Squash security fixes
mm-video-v4l2: Avoid buffer access after free buffer call
Change-Id: Ifde8d4e170b8dbeb9f7485d0222b05c3b2a960f3
mm-video-v4l2: venc: Use client allocated memory if available
Change-Id: I45e4f117e98588ee7c888ec5c1cb2424bc7e5fa3
mm-video-v4l2: venc: Avoid buffer access after free
Change-Id: Id439aac54ee64a65ea68b6431a9f5150255a6980
Bugs Fixed: 62452543, 36130225,64750179
CRs-Fixed: 2106434, 2115779
Change-Id: Ifde8d4e170b8dbeb9f7485d0222b05c3b2a960f3
Diffstat (limited to 'mm-video-v4l2')
-rw-r--r-- | mm-video-v4l2/vidc/venc/inc/omx_video_base.h | 4 | ||||
-rw-r--r-- | mm-video-v4l2/vidc/venc/src/omx_video_base.cpp | 53 | ||||
-rw-r--r-- | mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp | 9 |
3 files changed, 49 insertions, 17 deletions
diff --git a/mm-video-v4l2/vidc/venc/inc/omx_video_base.h b/mm-video-v4l2/vidc/venc/inc/omx_video_base.h index 233c9b4e..0027694d 100644 --- a/mm-video-v4l2/vidc/venc/inc/omx_video_base.h +++ b/mm-video-v4l2/vidc/venc/inc/omx_video_base.h @@ -757,6 +757,8 @@ 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_client_in_bm_count; uint64_t m_inp_bm_count; // bitmask array size for extradata uint64_t m_out_extradata_bm_count; @@ -770,6 +772,8 @@ class omx_video: public qc_omx_component bool hw_overload; size_t m_graphicbuffer_size; char m_platform[OMX_MAX_STRINGNAME_SIZE]; + + bool m_buffer_freed; }; #endif // __OMX_VIDEO_BASE_H__ diff --git a/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp b/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp index ed6c41cc..00072fc2 100644 --- a/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp +++ b/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp @@ -260,6 +260,8 @@ omx_video::omx_video(): pending_output_buffers(0), 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_out_extradata_bm_count(0), m_flags(0), @@ -267,7 +269,8 @@ omx_video::omx_video(): m_fbd_count(0), m_event_port_settings_sent(false), hw_overload(false), - m_graphicbuffer_size(0) + m_graphicbuffer_size(0), + m_buffer_freed(0) { DEBUG_PRINT_HIGH("omx_video(): Inside Constructor()"); memset(&m_cmp,0,sizeof(m_cmp)); @@ -404,6 +407,9 @@ void omx_video::process_event_cb(void *ctxt) case OMX_CommandStateSet: pThis->m_state = (OMX_STATETYPE) p2; DEBUG_PRINT_LOW("Process -> state set to %d", pThis->m_state); + if (pThis->m_state == OMX_StateLoaded) { + m_buffer_freed = false; + } pThis->m_pCallbacks.EventHandler(&pThis->m_cmp, pThis->m_app_data, OMX_EventCmdComplete, p1, p2, NULL); break; @@ -2602,6 +2608,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); @@ -2722,7 +2729,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; @@ -2860,6 +2866,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); @@ -2897,8 +2904,9 @@ 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); } else if (port == PORT_INDEX_OUT) { eRet = use_output_buffer(hComp,bufferHdr,port,appData,bytes,buffer); @@ -2908,7 +2916,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)) { @@ -3056,7 +3063,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) { @@ -3590,10 +3596,9 @@ 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); #ifdef _ANDROID_ICS_ if (meta_mode_enable) eRet = allocate_input_meta_buffer(hComp,bufferHdr,appData,bytes); @@ -3662,7 +3667,16 @@ 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); + } 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))) { DEBUG_PRINT_LOW(" free buffer while Component in Loading pending"); @@ -3671,12 +3685,14 @@ OMX_ERRORTYPE omx_video::free_buffer(OMX_IN OMX_HANDLETYPE hComp, DEBUG_PRINT_LOW("Free Buffer while port %u disabled", (unsigned int)port); } else if (m_state == OMX_StateExecuting || m_state == OMX_StatePause) { DEBUG_PRINT_ERROR("ERROR: Invalid state to free buffer,ports need to be disabled"); + m_buffer_freed = true; post_event(OMX_EventError, OMX_ErrorPortUnpopulated, OMX_COMPONENT_GENERATE_EVENT); return eRet; } else { DEBUG_PRINT_ERROR("ERROR: Invalid state to free buffer,port lost Buffers"); + m_buffer_freed = true; post_event(OMX_EventError, OMX_ErrorPortUnpopulated, OMX_COMPONENT_GENERATE_EVENT); @@ -3688,12 +3704,10 @@ 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; @@ -3721,7 +3735,6 @@ 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; } @@ -3742,7 +3755,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; @@ -3811,6 +3823,9 @@ OMX_ERRORTYPE omx_video::free_buffer(OMX_IN OMX_HANDLETYPE hComp, m_out_bm_count, m_inp_bm_count); } } + if (eRet != OMX_ErrorNone) { + m_buffer_freed = true; + } return eRet; } @@ -4056,9 +4071,9 @@ 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)) { + if (pmem_data_buf && BITMASK_PRESENT(&m_client_in_bm_count, nBufIndex)) { memcpy (pmem_data_buf, (buffer->pBuffer + buffer->nOffset), buffer->nFilledLen); } @@ -4177,9 +4192,15 @@ OMX_ERRORTYPE omx_video::fill_this_buffer_proxy( (void)hComp; OMX_U8 *pmem_data_buf = NULL; OMX_ERRORTYPE nRet = OMX_ErrorNone; + auto_lock l(m_buf_lock); + if (m_buffer_freed == true) { + DEBUG_PRINT_ERROR("ERROR: FTBProxy: Invalid call. Called after freebuffer"); + return OMX_ErrorBadParameter; + } - DEBUG_PRINT_LOW("FTBProxy: bufferAdd->pBuffer[%p]", bufferAdd->pBuffer); - + if (bufferAdd != NULL) { + DEBUG_PRINT_LOW("FTBProxy: bufferAdd->pBuffer[%p]", bufferAdd->pBuffer); + } if (bufferAdd == NULL || ((bufferAdd - m_out_mem_ptr) >= (int)m_sOutPortDef.nBufferCountActual) ) { DEBUG_PRINT_ERROR("ERROR: FTBProxy: Invalid i/p params"); return OMX_ErrorBadParameter; diff --git a/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp b/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp index 3fc8171d..540f8b5a 100644 --- a/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp +++ b/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp @@ -2082,11 +2082,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]); } @@ -2108,6 +2112,8 @@ OMX_ERRORTYPE omx_venc::component_deinit(OMX_IN OMX_HANDLETYPE hComp) for (i=0; i<m_sInPortDef.nBufferCountActual; i++ ) { if (BITMASK_PRESENT(&m_inp_bm_count, i)) { BITMASK_CLEAR(&m_inp_bm_count, i); + if (BITMASK_PRESENT(&m_client_in_bm_count, i)) + BITMASK_CLEAR(&m_client_in_bm_count, i); free_input_buffer (&m_inp_mem_ptr[i]); } @@ -2443,7 +2449,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), |