mm-video-v4l2: venc: Squash below changesandroid-security-8.1.0_r93android-security-8.1.0_r92android-security-8.1.0_r91android-security-8.1.0_r90android-security-8.1.0_r89android-security-8.1.0_r88android-security-8.1.0_r87android-security-8.1.0_r86android-security-8.1.0_r85android-security-8.1.0_r84android-security-8.1.0_r83android-security-8.1.0_r82android-8.1.0_r81android-8.1.0_r80android-8.1.0_r79android-8.1.0_r78android-8.1.0_r77android-8.1.0_r76android-8.1.0_r75android-8.1.0_r74android-8.1.0_r73android-8.1.0_r72android-8.1.0_r71android-8.1.0_r70android-8.1.0_r69android-8.1.0_r68android-8.1.0_r66security-oc-mr1-releaseoreo-mr1-security-release

mm-video-v4l2: venc: Protect buffer from being freed while accessing
Change-Id: I6141e81d7dbd50bc3601c8df066fd8cbd06b4e0b

mm-video-v4l2: Protect buffer lifecycle with lock
Change-Id: I0fdb4051c94044e032c257febbe2ba1c7e4d6c7e

mm-video-v4l2: venc: Avoid buffer access after free
Change-Id: Id439aac54ee64a65ea68b6431a9f5150255a6980

mm-video-v4l2: venc: Use client allocated memory if available
Change-Id: I45e4f117e98588ee7c888ec5c1cb2424bc7e5fa3

mm-video-v4l2: Avoid buffer access after free buffer call
Change-Id: Ifde8d4e170b8dbeb9f7485d0222b05c3b2a960f3

Bug:62452543 Bug:36130225 Bug:64750179
CRs-Fixed: 2062772, 2106434, 2106434, 2115779

Test: cts-tradefed run cts -m CtsMediaTestCases and CtsCameraTestCases
Change-Id: Ifde8d4e170b8dbeb9f7485d0222b05c3b2a960f3
(cherry picked from commit b74baf2c2a6532d890c0d97169d140f0a3d1fe37)

3 files changed, 62 insertions, 16 deletions

diff --git a/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h b/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h
index 8643e3f..7ea8fd7 100644
--- a/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h
+++ b/msm8998/mm-video-v4l2/vidc/venc/inc/omx_video_base.h
@@ -690,6 +690,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;
@@ -701,6 +702,8 @@ class omx_video: public qc_omx_component
         bool allocate_native_handle;
 
         uint64_t m_out_bm_count;
+        uint64_t m_client_out_bm_count;
+        uint64_t m_client_in_bm_count;
         uint64_t m_inp_bm_count;
         uint64_t m_flags;
         uint64_t m_etb_count;
@@ -713,6 +716,7 @@ class omx_video: public qc_omx_component
         bool hw_overload;
         size_t m_graphicbuffer_size;
         char m_platform[OMX_MAX_STRINGNAME_SIZE];
+        bool m_buffer_freed;
 };
 
 #endif // __OMX_VIDEO_BASE_H__
diff --git a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp
index c091162..ae640a7 100644
--- a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp
+++ b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_base.cpp
@@ -289,13 +289,16 @@ omx_video::omx_video():
     pending_output_buffers(0),
     allocate_native_handle(false),
     m_out_bm_count(0),
+    m_client_out_bm_count(0),
+    m_client_in_bm_count(0),
     m_inp_bm_count(0),
     m_flags(0),
     m_etb_count(0),
     m_fbd_count(0),
     m_event_port_settings_sent(false),
     hw_overload(false),
-    m_graphicbuffer_size(0)
+    m_graphicbuffer_size(0),
+    m_buffer_freed(0)
 {
     DEBUG_PRINT_HIGH("omx_video(): Inside Constructor()");
     memset(&m_cmp,0,sizeof(m_cmp));
@@ -320,6 +323,8 @@ omx_video::omx_video():
     property_get("ro.board.platform", platform_name, "0");
     strlcpy(m_platform, platform_name, sizeof(m_platform));
 #endif
+
+    pthread_mutex_init(&m_buf_lock, NULL);
 }
 
 
@@ -361,6 +366,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 ...");
 }
@@ -433,6 +440,9 @@ void omx_video::process_event_cb(void *ctxt, unsigned char id)
                             case OMX_CommandStateSet:
                                 pThis->m_state = (OMX_STATETYPE) p2;
                                 DEBUG_PRINT_LOW("Process -> state set to %d", pThis->m_state);
+                                if (pThis->m_state == OMX_StateLoaded) {
+                                    m_buffer_freed = false;
+                                }
                                 pThis->m_pCallbacks.EventHandler(&pThis->m_cmp, pThis->m_app_data,
                                         OMX_EventCmdComplete, p1, p2, NULL);
                                 break;
@@ -2619,6 +2629,7 @@ OMX_ERRORTYPE  omx_video::use_input_buffer(
 
         *bufferHdr = (m_inp_mem_ptr + i);
         BITMASK_SET(&m_inp_bm_count,i);
+        BITMASK_SET(&m_client_in_bm_count,i);
 
         (*bufferHdr)->pBuffer           = (OMX_U8 *)buffer;
         (*bufferHdr)->nSize             = sizeof(OMX_BUFFERHEADERTYPE);
@@ -2897,6 +2908,7 @@ OMX_ERRORTYPE  omx_video::use_output_buffer(
             }
 
             BITMASK_SET(&m_out_bm_count,i);
+            BITMASK_SET(&m_client_out_bm_count,i);
         } else {
             DEBUG_PRINT_ERROR("ERROR: All o/p Buffers have been Used, invalid use_buf call for "
                     "index = %u", i);
@@ -2934,8 +2946,9 @@ OMX_ERRORTYPE  omx_video::use_buffer(
         DEBUG_PRINT_ERROR("ERROR: Use Buffer in Invalid State");
         return OMX_ErrorInvalidState;
     }
+
+    auto_lock l(m_buf_lock);
     if (port == PORT_INDEX_IN) {
-        auto_lock l(m_lock);
         eRet = use_input_buffer(hComp,bufferHdr,port,appData,bytes,buffer);
     } else if (port == PORT_INDEX_OUT) {
         eRet = use_output_buffer(hComp,bufferHdr,port,appData,bytes,buffer);
@@ -2943,7 +2956,6 @@ OMX_ERRORTYPE  omx_video::use_buffer(
         DEBUG_PRINT_ERROR("ERROR: Invalid Port Index received %d",(int)port);
         eRet = OMX_ErrorBadPortIndex;
     }
-
     if (eRet == OMX_ErrorNone) {
         if (allocate_done()) {
             if (BITMASK_PRESENT(&m_flags,OMX_COMPONENT_IDLE_PENDING)) {
@@ -3006,7 +3018,6 @@ OMX_ERRORTYPE omx_video::free_input_buffer(OMX_BUFFERHEADERTYPE *bufferHdr)
     }
 
     if (index < m_sInPortDef.nBufferCountActual && m_pInput_pmem) {
-        auto_lock l(m_lock);
 
         if (mUseProxyColorFormat) {
             if (m_opq_pmem_q.m_size) {
@@ -3560,10 +3571,9 @@ OMX_ERRORTYPE  omx_video::allocate_buffer(OMX_IN OMX_HANDLETYPE                h
         DEBUG_PRINT_ERROR("ERROR: Allocate Buf in Invalid State");
         return OMX_ErrorInvalidState;
     }
-
+     auto_lock l(m_buf_lock);
     // What if the client calls again.
     if (port == PORT_INDEX_IN) {
-        auto_lock l(m_lock);
 #ifdef _ANDROID_ICS_
         if (meta_mode_enable)
             eRet = allocate_input_meta_buffer(hComp,bufferHdr,appData,bytes);
@@ -3632,7 +3642,16 @@ OMX_ERRORTYPE  omx_video::free_buffer(OMX_IN OMX_HANDLETYPE         hComp,
     unsigned int nPortIndex;
 
     DEBUG_PRINT_LOW("In for encoder free_buffer");
-
+    auto_lock l(m_buf_lock);
+    if (port == PORT_INDEX_OUT) { //client called freebuffer, clearing client buffer bitmask right away to avoid use after free
+        nPortIndex = buffer - (OMX_BUFFERHEADERTYPE*)m_out_mem_ptr;
+        if(BITMASK_PRESENT(&m_client_out_bm_count, nPortIndex))
+            BITMASK_CLEAR(&m_client_out_bm_count,nPortIndex);
+    } else if (port == PORT_INDEX_IN) {
+        nPortIndex = buffer - (meta_mode_enable?meta_buffer_hdr:m_inp_mem_ptr);
+        if(BITMASK_PRESENT(&m_client_in_bm_count, nPortIndex))
+            BITMASK_CLEAR(&m_client_in_bm_count,nPortIndex);
+    }
     if (m_state == OMX_StateIdle &&
             (BITMASK_PRESENT(&m_flags ,OMX_COMPONENT_LOADING_PENDING))) {
         DEBUG_PRINT_LOW(" free buffer while Component in Loading pending");
@@ -3641,12 +3660,14 @@ OMX_ERRORTYPE  omx_video::free_buffer(OMX_IN OMX_HANDLETYPE         hComp,
         DEBUG_PRINT_LOW("Free Buffer while port %u disabled", (unsigned int)port);
     } else if (m_state == OMX_StateExecuting || m_state == OMX_StatePause) {
         DEBUG_PRINT_ERROR("ERROR: Invalid state to free buffer,ports need to be disabled");
+        m_buffer_freed = true;
         post_event(OMX_EventError,
                 OMX_ErrorPortUnpopulated,
                 OMX_COMPONENT_GENERATE_EVENT);
         return eRet;
     } else {
         DEBUG_PRINT_ERROR("ERROR: Invalid state to free buffer,port lost Buffers");
+        m_buffer_freed = true;
         post_event(OMX_EventError,
                 OMX_ErrorPortUnpopulated,
                 OMX_COMPONENT_GENERATE_EVENT);
@@ -3658,12 +3679,10 @@ OMX_ERRORTYPE  omx_video::free_buffer(OMX_IN OMX_HANDLETYPE         hComp,
 
         DEBUG_PRINT_LOW("free_buffer on i/p port - Port idx %u, actual cnt %u",
                 nPortIndex, (unsigned int)m_sInPortDef.nBufferCountActual);
-        pthread_mutex_lock(&m_lock);
         if (nPortIndex < m_sInPortDef.nBufferCountActual &&
                 BITMASK_PRESENT(&m_inp_bm_count, nPortIndex)) {
             // Clear the bit associated with it.
             BITMASK_CLEAR(&m_inp_bm_count,nPortIndex);
-            pthread_mutex_unlock(&m_lock);
             free_input_buffer (buffer);
             m_sInPortDef.bPopulated = OMX_FALSE;
 
@@ -3691,7 +3710,6 @@ OMX_ERRORTYPE  omx_video::free_buffer(OMX_IN OMX_HANDLETYPE         hComp,
 #endif
             }
         } else {
-            pthread_mutex_unlock(&m_lock);
             DEBUG_PRINT_ERROR("ERROR: free_buffer ,Port Index Invalid");
             eRet = OMX_ErrorBadPortIndex;
         }
@@ -3771,6 +3789,9 @@ OMX_ERRORTYPE  omx_video::free_buffer(OMX_IN OMX_HANDLETYPE         hComp,
                     m_out_bm_count, m_inp_bm_count);
         }
     }
+    if (eRet != OMX_ErrorNone) {
+        m_buffer_freed = true;
+    }
 
     return eRet;
 }
@@ -3991,9 +4012,9 @@ OMX_ERRORTYPE  omx_video::empty_this_buffer_proxy(OMX_IN OMX_HANDLETYPE  hComp,
     {
         DEBUG_PRINT_LOW("Heap UseBuffer case, so memcpy the data");
 
-        auto_lock l(m_lock);
+        auto_lock l(m_buf_lock);
         pmem_data_buf = (OMX_U8 *)m_pInput_pmem[nBufIndex].buffer;
-        if (pmem_data_buf && BITMASK_PRESENT(&m_inp_bm_count, nBufIndex)) {
+        if (pmem_data_buf && BITMASK_PRESENT(&m_client_in_bm_count, nBufIndex)) {
             memcpy (pmem_data_buf, (buffer->pBuffer + buffer->nOffset),
                     buffer->nFilledLen);
         }
@@ -4110,9 +4131,15 @@ OMX_ERRORTYPE  omx_video::fill_this_buffer_proxy(
     (void)hComp;
     OMX_U8 *pmem_data_buf = NULL;
     OMX_ERRORTYPE nRet = OMX_ErrorNone;
+    auto_lock l(m_buf_lock);
+    if (m_buffer_freed == true) {
+        DEBUG_PRINT_ERROR("ERROR: FTBProxy: Invalid call. Called after freebuffer");
+        return OMX_ErrorBadParameter;
+    }
 
-    DEBUG_PRINT_LOW("FTBProxy: bufferAdd->pBuffer[%p]", bufferAdd->pBuffer);
-
+    if (bufferAdd != NULL) {
+        DEBUG_PRINT_LOW("FTBProxy: bufferAdd->pBuffer[%p]", bufferAdd->pBuffer);
+    }
     if (bufferAdd == NULL || ((bufferAdd - m_out_mem_ptr) >= (int)m_sOutPortDef.nBufferCountActual) ) {
         DEBUG_PRINT_ERROR("ERROR: FTBProxy: Invalid i/p params");
         return OMX_ErrorBadParameter;
diff --git a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp
index f0468bf..526ebb4 100644
--- a/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp
+++ b/msm8998/mm-video-v4l2/vidc/venc/src/omx_video_encoder.cpp
@@ -2361,11 +2361,15 @@ OMX_ERRORTYPE  omx_venc::component_deinit(OMX_IN OMX_HANDLETYPE hComp)
         DEBUG_PRINT_ERROR("WARNING:Rxd DeInit,OMX not in LOADED state %d",\
                 m_state);
     }
+
+    auto_lock l(m_buf_lock);
     if (m_out_mem_ptr) {
         DEBUG_PRINT_LOW("Freeing the Output Memory");
         for (i=0; i< m_sOutPortDef.nBufferCountActual; i++ ) {
             if (BITMASK_PRESENT(&m_out_bm_count, i)) {
                 BITMASK_CLEAR(&m_out_bm_count, i);
+                if (BITMASK_PRESENT(&m_client_out_bm_count, i))
+                    BITMASK_CLEAR(&m_client_out_bm_count, i);
                 free_output_buffer (&m_out_mem_ptr[i]);
             }
 
@@ -2387,6 +2391,8 @@ OMX_ERRORTYPE  omx_venc::component_deinit(OMX_IN OMX_HANDLETYPE hComp)
         for (i=0; i<m_sInPortDef.nBufferCountActual; i++ ) {
             if (BITMASK_PRESENT(&m_inp_bm_count, i)) {
                 BITMASK_CLEAR(&m_inp_bm_count, i);
+                if (BITMASK_PRESENT(&m_client_in_bm_count, i))
+                    BITMASK_CLEAR(&m_client_in_bm_count, i);
                 free_input_buffer (&m_inp_mem_ptr[i]);
             }
 
@@ -2706,10 +2712,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;
@@ -2718,7 +2731,8 @@ int omx_venc::async_message_process (void *context, void* message)
                     omxhdr->nFlags = m_sVenc_msg->buf.flags;
 
                     /*Use buffer case*/
-                    if (omx->output_use_buffer && !omx->m_use_output_pmem && !omx->is_secure_session()) {
+                    if (BITMASK_PRESENT(&(omx->m_client_out_bm_count), bufIndex) &&
+                        omx->output_use_buffer && !omx->m_use_output_pmem && !omx->is_secure_session()) {
                         DEBUG_PRINT_LOW("memcpy() for o/p Heap UseBuffer");
                         memcpy(omxhdr->pBuffer,
                                 (m_sVenc_msg->buf.ptrbuffer),
@@ -2752,6 +2766,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;