From 305626d38e84f0501a52eaa53850faa10cb3e3b9 Mon Sep 17 00:00:00 2001
From: Paras Nagda <pnagda@codeaurora.org>
Date: Tue, 4 Jul 2017 07:43:00 -0600
Subject: BACKPORT: mm-video: 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
(cherry picked from commit b28bc2107701cd6386751af7109aee7ad607f8f7)
---
 mm-video/vidc/venc/inc/omx_video_base.h      |  1 +
 mm-video/vidc/venc/src/omx_video_base.cpp    |  5 +++++
 mm-video/vidc/venc/src/omx_video_encoder.cpp | 17 +++++++++++++----
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/mm-video/vidc/venc/inc/omx_video_base.h b/mm-video/vidc/venc/inc/omx_video_base.h
index d64d9675..ca698614 100644
--- a/mm-video/vidc/venc/inc/omx_video_base.h
+++ b/mm-video/vidc/venc/inc/omx_video_base.h
@@ -562,6 +562,7 @@ public:
   OMX_BUFFERHEADERTYPE meta_buffer_hdr[MAX_NUM_INPUT_BUFFERS];
   // Output memory pointer
   OMX_BUFFERHEADERTYPE  *m_out_mem_ptr;
+  pthread_mutex_t m_buf_lock;
 
   bool input_flush_progress;
   bool output_flush_progress;
diff --git a/mm-video/vidc/venc/src/omx_video_base.cpp b/mm-video/vidc/venc/src/omx_video_base.cpp
index e9d18069..2e665dc4 100644
--- a/mm-video/vidc/venc/src/omx_video_base.cpp
+++ b/mm-video/vidc/venc/src/omx_video_base.cpp
@@ -281,6 +281,8 @@ omx_video::omx_video(): m_state(OMX_StateInvalid),
   m_venc_num_instances++;
   DEBUG_PRINT_HIGH("Venc instance = %d, ion device fd = %d",
      m_venc_num_instances, m_venc_ion_devicefd);
+
+  pthread_mutex_init(&m_buf_lock, NULL);
 }
 
 
@@ -322,6 +324,7 @@ omx_video::~omx_video()
   }
   DEBUG_PRINT_HIGH("m_etb_count = %u, m_fbd_count = %u", m_etb_count,
       m_fbd_count);
+  pthread_mutex_destroy(&m_buf_lock);
   DEBUG_PRINT_HIGH("Exiting OMX Video Encoder ...\n");
 }
 
@@ -2354,6 +2357,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;
@@ -3323,6 +3327,7 @@ OMX_ERRORTYPE  omx_video::free_buffer(OMX_IN OMX_HANDLETYPE         hComp,
     if(nPortIndex < m_sOutPortDef.nBufferCountActual &&
        BITMASK_PRESENT_U32(m_out_bm_count,nPortIndex))
     {
+      auto_lock l(m_buf_lock);
       // Clear the bit associated with it.
       m_out_bm_count = BITMASK_CLEAR_U32(m_out_bm_count,nPortIndex);
       m_sOutPortDef.bPopulated = OMX_FALSE;
diff --git a/mm-video/vidc/venc/src/omx_video_encoder.cpp b/mm-video/vidc/venc/src/omx_video_encoder.cpp
index 864df487..32223fce 100644
--- a/mm-video/vidc/venc/src/omx_video_encoder.cpp
+++ b/mm-video/vidc/venc/src/omx_video_encoder.cpp
@@ -1877,13 +1877,21 @@ 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))
-    {
-      if(m_sVenc_msg->buf.len <=  omxhdr->nAllocLen)
+        (bufIndex  < omx->m_sOutPortDef.nBufferCountActual))
+      {
+      auto_lock l(omx->m_buf_lock);
+      if (BITMASK_ABSENT_U32(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))
       {
         int idx = omxhdr - omx->m_out_mem_ptr;
         omxhdr->nFilledLen = m_sVenc_msg->buf.len;
@@ -1927,6 +1935,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;
-- 
cgit v1.2.3