Don't leak this out of GraphicBufferSource ctor

Bug: 37622974 Bug: 37622987 Bug: 37623757 Test: run poc and observe no crash Change-Id: I1e25c011f02bec26a1480ec9a217a52f15d43cf2 (cherry picked from commit 6301f88)
android-security · Aug 26, 2017 · 368000f · 368000f
1 parent e4f91fa
commit 368000f
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 11 deletions.
diff --git a/media/libstagefright/omx/GraphicBufferSource.cpp b/media/libstagefright/omx/GraphicBufferSource.cpp
@@ -170,9 +170,12 @@ GraphicBufferSource::GraphicBufferSource(
         mIsPersistent = true;
     }
     mConsumer->setDefaultBufferSize(bufferWidth, bufferHeight);
-    // Note that we can't create an sp<...>(this) in a ctor that will not keep a
-    // reference once the ctor ends, as that would cause the refcount of 'this'
-    // dropping to 0 at the end of the ctor.  Since all we need is a wp<...>
+}
+
+status_t GraphicBufferSource::init() {
+    // Note that we can't create an sp<...>(this) in a method that will not keep a
+    // reference once the method ends, as that may cause the refcount of 'this'
+    // dropping to 0 at the end of the method.  Since all we need is a wp<...>
     // that's what we create.
     wp<BufferQueue::ConsumerListener> listener = static_cast<BufferQueue::ConsumerListener*>(this);
     sp<IConsumerListener> proxy;
@@ -186,10 +189,9 @@ GraphicBufferSource::GraphicBufferSource(
     if (mInitCheck != NO_ERROR) {
         ALOGE("Error connecting to BufferQueue: %s (%d)",
                 strerror(-mInitCheck), mInitCheck);
-        return;
     }
 
-    CHECK(mInitCheck == NO_ERROR);
+    return mInitCheck;
 }
 
 GraphicBufferSource::~GraphicBufferSource() {

diff --git a/media/libstagefright/omx/GraphicBufferSource.h b/media/libstagefright/omx/GraphicBufferSource.h
@@ -61,11 +61,7 @@ class GraphicBufferSource : public BufferQueue::ConsumerListener {
 
     virtual ~GraphicBufferSource();
 
-    // We can't throw an exception if the constructor fails, so we just set
-    // this and require that the caller test the value.
-    status_t initCheck() const {
-        return mInitCheck;
-    }
+    status_t init();
 
     // Returns the handle to the producer side of the BufferQueue.  Buffers
     // queued on this will be received by GraphicBufferSource.

diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp
@@ -1052,7 +1052,7 @@ status_t OMXNodeInstance::createGraphicBufferSource(
             usageBits,
             bufferConsumer);
 
-    if ((err = bufferSource->initCheck()) != OK) {
+    if ((err = bufferSource->init()) != OK) {
         return err;
     }
     setGraphicBufferSource(bufferSource);