summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPraveen Chavan <pchavan@codeaurora.org>2017-06-05 15:21:22 -0700
committerVasantha Balla <vballa@codeaurora.org>2017-07-04 17:31:05 +0530
commit22979e32dae095242cb203b72f946b8eb2eb7f78 (patch)
tree56dcbbcbec2a3a1162564a75dd29f534af17e83c
parent688bfd18c4423652bf1e92b12ecc23f5e6010434 (diff)
downloadandroid_hardware_qcom_media-22979e32dae095242cb203b72f946b8eb2eb7f78.tar.gz
android_hardware_qcom_media-22979e32dae095242cb203b72f946b8eb2eb7f78.tar.bz2
android_hardware_qcom_media-22979e32dae095242cb203b72f946b8eb2eb7f78.zip
mm-video-v4l2: venc: Protect buffer from being freed while accessing
Output buffer (in use-buffer mode) has an internal backup ion buffer. The contents of this buffer are deep-copied in client's buffer in the context of VideoEncCallBackThread; while this buffer can be freed in the client thread's context. Check the allocation bitmask before attempting to copy and synchronize these operations by holding a lock Fixes bug 36130225 Security Vulnerability - Heap use after free in libOmxVenc CRs-Fixed: 2053101 Change-Id: I6141e81d7dbd50bc3601c8df066fd8cbd06b4e0b
-rw-r--r--mm-video-v4l2/vidc/venc/inc/omx_video_base.h1
-rw-r--r--mm-video-v4l2/vidc/venc/src/omx_video_base.cpp6
-rw-r--r--mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp10
3 files changed, 16 insertions, 1 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 3daf08cd..08a43603 100644
--- a/mm-video-v4l2/vidc/venc/inc/omx_video_base.h
+++ b/mm-video-v4l2/vidc/venc/inc/omx_video_base.h
@@ -645,6 +645,7 @@ class omx_video: public qc_omx_component
omx_cmd_queue m_opq_meta_q;
omx_cmd_queue m_opq_pmem_q;
OMX_BUFFERHEADERTYPE meta_buffer_hdr[MAX_NUM_INPUT_BUFFERS];
+ pthread_mutex_t m_buf_lock;
bool input_flush_progress;
bool output_flush_progress;
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 ba071037..c3696ec2 100644
--- a/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp
+++ b/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp
@@ -258,6 +258,8 @@ omx_video::omx_video():
pthread_mutex_init(&m_lock, NULL);
sem_init(&m_cmd_lock,0,0);
DEBUG_PRINT_LOW("meta_buffer_hdr = %p", meta_buffer_hdr);
+
+ pthread_mutex_init(&m_buf_lock, NULL);
}
@@ -293,6 +295,8 @@ omx_video::~omx_video()
sem_destroy(&m_cmd_lock);
DEBUG_PRINT_HIGH("m_etb_count = %" PRIu64" , m_fbd_count = %" PRIu64, m_etb_count,
m_fbd_count);
+
+ pthread_mutex_destroy(&m_buf_lock);
DEBUG_PRINT_HIGH("omx_video: Destructor exit");
DEBUG_PRINT_HIGH("Exiting OMX Video Encoder ...");
}
@@ -2396,6 +2400,7 @@ 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;
@@ -3322,6 +3327,7 @@ 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;
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 b6e7fd87..c05a73af 100644
--- a/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp
+++ b/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp
@@ -2214,10 +2214,17 @@ int omx_venc::async_message_process (void *context, void* message)
OMX_COMPONENT_GENERATE_EBD);
break;
case VEN_MSG_OUTPUT_BUFFER_DONE:
+ {
omxhdr = (OMX_BUFFERHEADERTYPE*)m_sVenc_msg->buf.clientdata;
+ OMX_U32 bufIndex = (OMX_U32)(omxhdr - omx->m_out_mem_ptr);
if ( (omxhdr != NULL) &&
- ((OMX_U32)(omxhdr - omx->m_out_mem_ptr) < omx->m_sOutPortDef.nBufferCountActual)) {
+ (bufIndex < omx->m_sOutPortDef.nBufferCountActual)) {
+ auto_lock l(omx->m_buf_lock);
+ if (BITMASK_ABSENT(&(omx->m_out_bm_count), bufIndex)) {
+ DEBUG_PRINT_ERROR("Recieved FBD for buffer that is already freed !");
+ break;
+ }
if (!omx->is_secure_session() && (m_sVenc_msg->buf.len <= omxhdr->nAllocLen)) {
omxhdr->nFilledLen = m_sVenc_msg->buf.len;
omxhdr->nOffset = m_sVenc_msg->buf.offset;
@@ -2251,6 +2258,7 @@ int omx_venc::async_message_process (void *context, void* message)
omx->post_event ((unsigned long)omxhdr,m_sVenc_msg->statuscode,
OMX_COMPONENT_GENERATE_FBD);
break;
+ }
case VEN_MSG_NEED_OUTPUT_BUFFER:
//TBD what action needs to be done here??
break;