summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMahesh Lanka <mlanka@codeaurora.org>2016-04-10 19:04:12 +0530
committerJessica Wagantall <jwagantall@cyngn.com>2016-06-13 16:09:31 -0700
commit4360f08e802a312d88817f6ac2fc3784bde65e20 (patch)
treecc66a787cb948b5c61d1fbf6f73dfe4433f4d958
parentb88f7f6fc39836c633f8ea637267f9e7de03c260 (diff)
downloadandroid_hardware_qcom_media-4360f08e802a312d88817f6ac2fc3784bde65e20.tar.gz
android_hardware_qcom_media-4360f08e802a312d88817f6ac2fc3784bde65e20.tar.bz2
android_hardware_qcom_media-4360f08e802a312d88817f6ac2fc3784bde65e20.zip
mm-video-v4l2: vdec: add safety checks for freeing buffers
Allow only up to 64 buffers on input/output port (since the allocation bitmap is only 64-wide). Do not allow changing theactual buffer count while still holding allocation (Client can technically negotiate buffer count on a free/disabled port) Add safety checks to free only as many buffers were allocated. Fixes: Security Vulnerability - Heap Overflow and Possible Local Privilege Escalation in MediaServer (libOmxVdec problem #3) Ticket: CYNGNOS-2707 Bug: 27532282 27661749 Change-Id: I06dd680d43feaef3efdc87311e8a6703e234b523 (cherry picked from commit 6313abe468c2f6da4dfad72dd8914c4a8c8902fb)
-rw-r--r--[-rwxr-xr-x]mm-video-v4l2/vidc/vdec/inc/omx_vdec.h2
-rw-r--r--mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp54
2 files changed, 45 insertions, 11 deletions
diff --git a/mm-video-v4l2/vidc/vdec/inc/omx_vdec.h b/mm-video-v4l2/vidc/vdec/inc/omx_vdec.h
index 59fd8367..ef10e748 100755..100644
--- a/mm-video-v4l2/vidc/vdec/inc/omx_vdec.h
+++ b/mm-video-v4l2/vidc/vdec/inc/omx_vdec.h
@@ -181,7 +181,7 @@ class VideoHeap : public MemoryHeapBase
#define DESC_BUFFER_SIZE (8192 * 16)
#ifdef _ANDROID_
-#define MAX_NUM_INPUT_OUTPUT_BUFFERS 32
+#define MAX_NUM_INPUT_OUTPUT_BUFFERS 64
#endif
#ifdef _ION_HEAP_MASK_COMPATIBILITY_WA
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 7665f478..3a6e72dd 100644
--- a/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp
+++ b/mm-video-v4l2/vidc/vdec/src/omx_vdec_msm8974.cpp
@@ -3109,7 +3109,11 @@ OMX_ERRORTYPE omx_vdec::set_parameter(OMX_IN OMX_HANDLETYPE hComp,
eRet = get_buffer_req(&drv_ctx.op_buf);
}
}
- if (!client_buffers.get_buffer_req(buffer_size)) {
+ if (portDefn->nBufferCountActual > MAX_NUM_INPUT_OUTPUT_BUFFERS) {
+ DEBUG_PRINT_ERROR("Requested o/p buf count (%u) exceeds limit (%u)",
+ portDefn->nBufferCountActual, MAX_NUM_INPUT_OUTPUT_BUFFERS);
+ eRet = OMX_ErrorBadParameter;
+ } else if (!client_buffers.get_buffer_req(buffer_size)) {
DEBUG_PRINT_ERROR("Error in getting buffer requirements");
eRet = OMX_ErrorBadParameter;
} else {
@@ -3211,6 +3215,19 @@ OMX_ERRORTYPE omx_vdec::set_parameter(OMX_IN OMX_HANDLETYPE hComp,
eRet = get_buffer_req(&drv_ctx.op_buf);
}
}
+ if (portDefn->nBufferCountActual > MAX_NUM_INPUT_OUTPUT_BUFFERS) {
+ DEBUG_PRINT_ERROR("Requested i/p buf count (%u) exceeds limit (%u)",
+ portDefn->nBufferCountActual, MAX_NUM_INPUT_OUTPUT_BUFFERS);
+ eRet = OMX_ErrorBadParameter;
+ break;
+ }
+ // Buffer count can change only when port is disabled
+ if (!release_input_done()) {
+ DEBUG_PRINT_ERROR("Cannot change i/p buffer count since all buffers are not freed yet !");
+ eRet = OMX_ErrorInvalidState;
+ break;
+ }
+
if (portDefn->nBufferCountActual >= drv_ctx.ip_buf.mincount
|| portDefn->nBufferSize != drv_ctx.ip_buf.buffer_size) {
port_format_changed = true;
@@ -5397,7 +5414,8 @@ OMX_ERRORTYPE omx_vdec::free_buffer(OMX_IN OMX_HANDLETYPE hComp,
nPortIndex = buffer - m_inp_heap_ptr;
DEBUG_PRINT_LOW("free_buffer on i/p port - Port idx %d", nPortIndex);
- if (nPortIndex < drv_ctx.ip_buf.actualcount) {
+ if (nPortIndex < drv_ctx.ip_buf.actualcount &&
+ BITMASK_PRESENT(&m_inp_bm_count, nPortIndex)) {
// Clear the bit associated with it.
BITMASK_CLEAR(&m_inp_bm_count,nPortIndex);
BITMASK_CLEAR(&m_heap_inp_bm_count,nPortIndex);
@@ -5439,7 +5457,8 @@ OMX_ERRORTYPE omx_vdec::free_buffer(OMX_IN OMX_HANDLETYPE hComp,
} else if (port == OMX_CORE_OUTPUT_PORT_INDEX) {
// check if the buffer is valid
nPortIndex = buffer - client_buffers.get_il_buf_hdr();
- if (nPortIndex < drv_ctx.op_buf.actualcount) {
+ if (nPortIndex < drv_ctx.op_buf.actualcount &&
+ BITMASK_PRESENT(&m_out_bm_count, nPortIndex)) {
DEBUG_PRINT_LOW("free_buffer on o/p port - Port idx %d", nPortIndex);
// Clear the bit associated with it.
BITMASK_CLEAR(&m_out_bm_count,nPortIndex);
@@ -6091,7 +6110,14 @@ OMX_ERRORTYPE omx_vdec::component_deinit(OMX_IN OMX_HANDLETYPE hComp)
if (m_out_mem_ptr) {
DEBUG_PRINT_LOW("Freeing the Output Memory");
for (i = 0; i < drv_ctx.op_buf.actualcount; i++ ) {
- free_output_buffer (&m_out_mem_ptr[i]);
+ if (BITMASK_PRESENT(&m_out_bm_count, i)) {
+ BITMASK_CLEAR(&m_out_bm_count, i);
+ client_buffers.free_output_buffer (&m_out_mem_ptr[i]);
+ }
+
+ if (release_output_done()) {
+ break;
+ }
}
#ifdef _ANDROID_ICS_
memset(&native_buffer, 0, (sizeof(nativebuffer) * MAX_NUM_INPUT_OUTPUT_BUFFERS));
@@ -6102,11 +6128,19 @@ OMX_ERRORTYPE omx_vdec::component_deinit(OMX_IN OMX_HANDLETYPE hComp)
if (m_inp_mem_ptr || m_inp_heap_ptr) {
DEBUG_PRINT_LOW("Freeing the Input Memory");
for (i = 0; i<drv_ctx.ip_buf.actualcount; i++ ) {
- if (m_inp_mem_ptr)
- free_input_buffer (i,&m_inp_mem_ptr[i]);
- else
- free_input_buffer (i,NULL);
- }
+
+ if (BITMASK_PRESENT(&m_inp_bm_count, i)) {
+ BITMASK_CLEAR(&m_inp_bm_count, i);
+ if (m_inp_mem_ptr)
+ free_input_buffer (i,&m_inp_mem_ptr[i]);
+ else
+ free_input_buffer (i,NULL);
+ }
+
+ if (release_input_done()) {
+ break;
+ }
+ }
}
free_input_buffer_header();
free_output_buffer_header();
@@ -6508,7 +6542,7 @@ bool omx_vdec::release_output_done(void)
bool bRet = false;
unsigned i=0,j=0;
- DEBUG_PRINT_LOW("Value of m_out_mem_ptr %p",m_inp_mem_ptr);
+ DEBUG_PRINT_LOW("Value of m_out_mem_ptr %p",m_out_mem_ptr);
if (m_out_mem_ptr) {
for (; j < drv_ctx.op_buf.actualcount ; j++) {
if (BITMASK_PRESENT(&m_out_bm_count,j)) {