diff options
author | Praveen Chavan <pchavan@codeaurora.org> | 2016-08-17 12:19:29 -0700 |
---|---|---|
committer | Ray Essick <essick@google.com> | 2016-09-23 16:10:44 -0700 |
commit | ee06e61ff49357884de5c6714828c263966895ee (patch) | |
tree | 6fdd185b422f470447ef36ba9d9a9e5f4656651e /msm8974/mm-video-v4l2/vidc | |
parent | f89f2c65e17c4f6df0845ac099e9197af317283e (diff) | |
download | media-ee06e61ff49357884de5c6714828c263966895ee.tar.gz |
mm-video-v4l2: vdec: Disallow changing buffer modes/counts on allocated ports
Changing Count, size, usage-mode (metadata/bytebuffer/native-handle) or
allocation-mode (allocateBuffer/UseBuffer) of buffers should only be
allowed when the port hasn't been allocated yet.
Since buffer-modes determine the payload-size in case of meta-buffer-mode,
and also determine the memory-base to derive buffer indices from buffer-
headers, letting the client change count/size/mode on a pre-allocated port
will cause inconsistencies in the size of memory allocated for headers and
lead to index overflows.
Fix the range checks for the derived buffer-indices to avoid out-of-bounds
writes.
Also, ensure buffer-mode settings (metadata-mode, native-handle-mode)
are intended for the right ports.
Bug: 29617572 : Heap Overflow/LPE in MediaServer (libOmxVdec problem #8)
Bug: 29982686 : Memory Write/LPE in MediaServer (libOmxVdec problem #10)
Change-Id: I619636a48779580c247bffb3752c3e4025b46542
Diffstat (limited to 'msm8974/mm-video-v4l2/vidc')
-rw-r--r-- | msm8974/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp | 86 |
1 files changed, 76 insertions, 10 deletions
diff --git a/msm8974/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp b/msm8974/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp index 1f898a3..dad7b4a 100644 --- a/msm8974/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp +++ b/msm8974/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp @@ -3303,6 +3303,13 @@ OMX_ERRORTYPE omx_vdec::set_parameter(OMX_IN OMX_HANDLETYPE hComp, DEBUG_PRINT_LOW("set_parameter: OMX_IndexParamPortDefinition H= %d, W = %d", (int)portDefn->format.video.nFrameHeight, (int)portDefn->format.video.nFrameWidth); + + if (portDefn->nBufferCountActual > MAX_NUM_INPUT_OUTPUT_BUFFERS) { + DEBUG_PRINT_ERROR("ERROR: Buffers requested exceeds max limit %d", + portDefn->nBufferCountActual); + eRet = OMX_ErrorBadParameter; + break; + } if (OMX_DirOutput == portDefn->eDir) { DEBUG_PRINT_LOW("set_parameter: OMX_IndexParamPortDefinition OP port"); bool port_format_changed = false; @@ -3413,8 +3420,11 @@ OMX_ERRORTYPE omx_vdec::set_parameter(OMX_IN OMX_HANDLETYPE hComp, eRet = OMX_ErrorBadParameter; } else if (!port_format_changed) { - // Buffer count can change only when port is disabled - if (!release_output_done()) { + // Buffer count can change only when port is unallocated + if (m_out_mem_ptr && + (portDefn->nBufferCountActual != drv_ctx.op_buf.actualcount || + portDefn->nBufferSize != drv_ctx.op_buf.buffer_size)) { + DEBUG_PRINT_ERROR("Cannot change o/p buffer count since all buffers are not freed yet !"); eRet = OMX_ErrorInvalidState; break; @@ -3534,8 +3544,10 @@ OMX_ERRORTYPE omx_vdec::set_parameter(OMX_IN OMX_HANDLETYPE hComp, eRet = OMX_ErrorBadParameter; break; } - // Buffer count can change only when port is disabled - if (!release_input_done()) { + // Buffer count can change only when port is unallocated + if (m_inp_mem_ptr && + (portDefn->nBufferCountActual != drv_ctx.ip_buf.actualcount || + portDefn->nBufferSize != drv_ctx.ip_buf.buffer_size)) { DEBUG_PRINT_ERROR("Cannot change i/p buffer count since all buffers are not freed yet !"); eRet = OMX_ErrorInvalidState; break; @@ -3619,6 +3631,14 @@ OMX_ERRORTYPE omx_vdec::set_parameter(OMX_IN OMX_HANDLETYPE hComp, /* Input port */ if (portFmt->nPortIndex == 0) { + // arbitrary_bytes mode cannot be changed arbitrarily since this controls how: + // - headers are allocated and + // - headers-indices are derived + // Avoid changing arbitrary_bytes when the port is already allocated + if (m_inp_mem_ptr) { + DEBUG_PRINT_ERROR("Cannot change arbitrary-bytes-mode since input port is not free!"); + return OMX_ErrorUnsupportedSetting; + } if (portFmt->nFramePackingFormat == OMX_QCOM_FramePacking_Arbitrary) { if (secure_mode) { arbitrary_bytes = false; @@ -3965,6 +3985,15 @@ OMX_ERRORTYPE omx_vdec::set_parameter(OMX_IN OMX_HANDLETYPE hComp, case OMX_GoogleAndroidIndexEnableAndroidNativeBuffers: { VALIDATE_OMX_PARAM_DATA(paramData, EnableAndroidNativeBuffersParams); EnableAndroidNativeBuffersParams* enableNativeBuffers = (EnableAndroidNativeBuffersParams *) paramData; + if (enableNativeBuffers->nPortIndex != OMX_CORE_OUTPUT_PORT_INDEX) { + DEBUG_PRINT_ERROR("Enable/Disable android-native-buffers allowed only on output port!"); + eRet = OMX_ErrorUnsupportedSetting; + break; + } else if (m_out_mem_ptr) { + DEBUG_PRINT_ERROR("Enable/Disable android-native-buffers is not allowed since Output port is not free !"); + eRet = OMX_ErrorInvalidState; + break; + } if (enableNativeBuffers) { m_enable_android_native_buffers = enableNativeBuffers->enable; } @@ -3985,7 +4014,20 @@ OMX_ERRORTYPE omx_vdec::set_parameter(OMX_IN OMX_HANDLETYPE hComp, } break; case OMX_GoogleAndroidIndexAllocateNativeHandle: { + AllocateNativeHandleParams* allocateNativeHandleParams = (AllocateNativeHandleParams *) paramData; + VALIDATE_OMX_PARAM_DATA(paramData, AllocateNativeHandleParams); + + if (allocateNativeHandleParams->nPortIndex != OMX_CORE_INPUT_PORT_INDEX) { + DEBUG_PRINT_ERROR("Enable/Disable allocate-native-handle allowed only on input port!"); + eRet = OMX_ErrorUnsupportedSetting; + break; + } else if (m_inp_mem_ptr) { + DEBUG_PRINT_ERROR("Enable/Disable allocate-native-handle is not allowed since Input port is not free !"); + eRet = OMX_ErrorInvalidState; + break; + } + if (allocateNativeHandleParams != NULL) { allocate_native_handle = allocateNativeHandleParams->enable; } @@ -4036,6 +4078,12 @@ OMX_ERRORTYPE omx_vdec::set_parameter(OMX_IN OMX_HANDLETYPE hComp, break; } if (metabuffer->nPortIndex == OMX_CORE_OUTPUT_PORT_INDEX) { + + if (m_out_mem_ptr) { + DEBUG_PRINT_ERROR("Enable/Disable dynamic-buffer-mode is not allowed since Output port is not free !"); + eRet = OMX_ErrorInvalidState; + break; + } //set property dynamic buffer mode to driver. struct v4l2_control control; struct v4l2_format fmt; @@ -5030,9 +5078,16 @@ OMX_ERRORTYPE omx_vdec::use_buffer( DEBUG_PRINT_ERROR("Use Buffer in Invalid State"); return OMX_ErrorInvalidState; } - if (port == OMX_CORE_INPUT_PORT_INDEX) + if (port == OMX_CORE_INPUT_PORT_INDEX) { + // If this is not the first allocation (i.e m_inp_mem_ptr is allocated), + // ensure that use-buffer was called for previous allocation. + // Mix-and-match of useBuffer and allocateBuffer is not allowed + if (m_inp_mem_ptr && !input_use_buffer) { + DEBUG_PRINT_ERROR("'Use' Input buffer called after 'Allocate' Input buffer !"); + return OMX_ErrorUndefined; + } error = use_input_heap_buffers(hComp, bufferHdr, port, appData, bytes, buffer); - else if (port == OMX_CORE_OUTPUT_PORT_INDEX) + } else if (port == OMX_CORE_OUTPUT_PORT_INDEX) error = use_output_buffer(hComp,bufferHdr,port,appData,bytes,buffer); //not tested else { DEBUG_PRINT_ERROR("Error: Invalid Port Index received %d",(int)port); @@ -5833,6 +5888,13 @@ OMX_ERRORTYPE omx_vdec::allocate_buffer(OMX_IN OMX_HANDLETYPE hC } if (port == OMX_CORE_INPUT_PORT_INDEX) { + // If this is not the first allocation (i.e m_inp_mem_ptr is allocated), + // ensure that use-buffer was never called. + // Mix-and-match of useBuffer and allocateBuffer is not allowed + if (m_inp_mem_ptr && input_use_buffer) { + DEBUG_PRINT_ERROR("'Allocate' Input buffer called after 'Use' Input buffer !"); + return OMX_ErrorUndefined; + } if (arbitrary_bytes) { eRet = allocate_input_heap_buffer (hComp,bufferHdr,port,appData,bytes); } else { @@ -6091,6 +6153,10 @@ OMX_ERRORTYPE omx_vdec::empty_this_buffer(OMX_IN OMX_HANDLETYPE hComp, } else { if (input_use_buffer == true) { nBufferIndex = buffer - m_inp_heap_ptr; + if (nBufferIndex >= drv_ctx.ip_buf.actualcount ) { + DEBUG_PRINT_ERROR("ERROR: ETB nBufferIndex is invalid in use-buffer mode"); + return OMX_ErrorBadParameter; + } m_inp_mem_ptr[nBufferIndex].nFilledLen = m_inp_heap_ptr[nBufferIndex].nFilledLen; m_inp_mem_ptr[nBufferIndex].nTimeStamp = m_inp_heap_ptr[nBufferIndex].nTimeStamp; m_inp_mem_ptr[nBufferIndex].nFlags = m_inp_heap_ptr[nBufferIndex].nFlags; @@ -6102,7 +6168,7 @@ OMX_ERRORTYPE omx_vdec::empty_this_buffer(OMX_IN OMX_HANDLETYPE hComp, } } - if (nBufferIndex > drv_ctx.ip_buf.actualcount ) { + if (nBufferIndex >= drv_ctx.ip_buf.actualcount ) { DEBUG_PRINT_ERROR("ERROR:ETB nBufferIndex is invalid"); return OMX_ErrorBadParameter; } @@ -6159,7 +6225,7 @@ OMX_ERRORTYPE omx_vdec::empty_this_buffer_proxy(OMX_IN OMX_HANDLETYPE hComp, nPortIndex = buffer-((OMX_BUFFERHEADERTYPE *)m_inp_mem_ptr); - if (nPortIndex > drv_ctx.ip_buf.actualcount) { + if (nPortIndex >= drv_ctx.ip_buf.actualcount) { DEBUG_PRINT_ERROR("ERROR:empty_this_buffer_proxy invalid nPortIndex[%u]", nPortIndex); return OMX_ErrorBadParameter; @@ -6480,7 +6546,7 @@ OMX_ERRORTYPE omx_vdec::fill_this_buffer_proxy( nPortIndex = buffer-((OMX_BUFFERHEADERTYPE *)client_buffers.get_il_buf_hdr()); - if (!bufferAdd || !bufferAdd->pBuffer || nPortIndex > drv_ctx.op_buf.actualcount) { + if (!bufferAdd || !bufferAdd->pBuffer || nPortIndex >= drv_ctx.op_buf.actualcount) { DEBUG_PRINT_ERROR("FTBProxy: ERROR: invalid buffer index, nPortIndex %u bufCount %u", nPortIndex, drv_ctx.op_buf.actualcount); return OMX_ErrorBadParameter; @@ -7392,7 +7458,7 @@ OMX_ERRORTYPE omx_vdec::empty_buffer_done(OMX_HANDLETYPE hComp, OMX_BUFFERHEADERTYPE* buffer) { - if (buffer == NULL || ((buffer - m_inp_mem_ptr) > (int)drv_ctx.ip_buf.actualcount)) { + if (buffer == NULL || ((buffer - m_inp_mem_ptr) >= (int)drv_ctx.ip_buf.actualcount)) { DEBUG_PRINT_ERROR("empty_buffer_done: ERROR bufhdr = %p", buffer); return OMX_ErrorBadParameter; } |