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
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 @@
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 @@
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 @@
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 @@
/* 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 @@
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 @@
}
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 @@
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 @@
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 @@
}
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 @@
} 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 @@
}
}
- 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 @@
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 @@
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_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;
}
diff --git a/msm8996/mm-video-v4l2/vidc/vdec/src/omx_vdec_v4l2.cpp b/msm8996/mm-video-v4l2/vidc/vdec/src/omx_vdec_v4l2.cpp
index 0fc2154..6eb0fb4 100644
--- a/msm8996/mm-video-v4l2/vidc/vdec/src/omx_vdec_v4l2.cpp
+++ b/msm8996/mm-video-v4l2/vidc/vdec/src/omx_vdec_v4l2.cpp
@@ -3875,7 +3875,7 @@
(int)portDefn->format.video.nFrameHeight,
(int)portDefn->format.video.nFrameWidth);
- if (portDefn->nBufferCountActual >= MAX_NUM_INPUT_OUTPUT_BUFFERS) {
+ if (portDefn->nBufferCountActual > MAX_NUM_INPUT_OUTPUT_BUFFERS) {
DEBUG_PRINT_ERROR("ERROR: Buffers requested exceeds max limit %d",
portDefn->nBufferCountActual);
eRet = OMX_ErrorBadParameter;
@@ -3995,8 +3995,11 @@
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;
@@ -4116,8 +4119,10 @@
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;
@@ -4210,6 +4215,14 @@
/* 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 || m_input_pass_buffer_fd) {
arbitrary_bytes = false;
@@ -4569,6 +4582,15 @@
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;
}
@@ -4633,6 +4655,12 @@
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;
@@ -5769,9 +5797,16 @@
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);
@@ -6560,6 +6595,13 @@
}
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 {
@@ -6809,6 +6851,10 @@
} 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;
@@ -6820,7 +6866,7 @@
}
}
- 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;
}
@@ -6882,7 +6928,7 @@
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;
@@ -7202,7 +7248,7 @@
nPortIndex = buffer-((OMX_BUFFERHEADERTYPE *)client_buffers.get_il_buf_hdr());
- if (bufferAdd == NULL || nPortIndex > drv_ctx.op_buf.actualcount) {
+ if (bufferAdd == NULL || 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;
@@ -8107,7 +8153,7 @@
int nBufferIndex = buffer - m_inp_mem_ptr;
- if (buffer == NULL || (nBufferIndex > (int)drv_ctx.ip_buf.actualcount)) {
+ if (buffer == NULL || (nBufferIndex >= (int)drv_ctx.ip_buf.actualcount)) {
DEBUG_PRINT_ERROR("empty_buffer_done: ERROR bufhdr = %p", buffer);
return OMX_ErrorBadParameter;
}
