commit fe3708f7baa75b37e7b1ac30f88acb33df3f5360
author Dongwon Kang <dwkang@google.com> Wed Jun 20 16:05:45 2018 -0700
committer Dongwon Kang <dwkang@google.com> Thu Jun 21 00:01:54 2018 +0000
tree eedb1c4a03292d35da38f6f4ae519ca0d4b79700
parent ed3f73e92a9aee279b60e174b443fd08bac2b847

Fix a race condition in OMXNodeInstance

When it frees the buffers, there might be pending events in
OMX::CallbackDispatcher and these event can be handled after
the component frees the buffers. To prevent the UAF case, this
change invalidates the buffers in the client side first before
calling OMX_FreeBuffer.

Test: run poc with and without the patch
Test: cts-tradefed run cts-dev --module CtsMediaTestCases
      --compatibility:module-arg CtsMediaTestCases:include-annotation:
      android.platform.test.annotations.RequiresDevice
Bug: 77474014
Change-Id: I0b7c4291967564f697e7f6a5ecbc31d4dae3cbcd

media/libstagefright/omx/OMXNodeInstance.cpp

diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp
index 6fdc2a1..28ba7a5 100644
--- a/media/libstagefright/omx/OMXNodeInstance.cpp
+++ b/media/libstagefright/omx/OMXNodeInstance.cpp
@@ -1249,12 +1249,15 @@
     }
     BufferMeta *buffer_meta = static_cast<BufferMeta *>(header->pAppPrivate);
 
+    // Invalidate buffers in the client side first before calling OMX_FreeBuffer.
+    // If not, pending events in the client side might access the buffers after free.
+    invalidateBufferID(buffer);
+
     OMX_ERRORTYPE err = OMX_FreeBuffer(mHandle, portIndex, header);
     CLOG_IF_ERROR(freeBuffer, err, "%s:%u %#x", portString(portIndex), portIndex, buffer);
 
     delete buffer_meta;
     buffer_meta = NULL;
-    invalidateBufferID(buffer);
 
     return StatusFromOMXError(err);
 }