commit 58da77ea4b7be4ecb0fee10d20b0e405de7eb2be
author Steven Thomas <steventhomas@google.com> Wed Apr 26 15:24:28 2017 -0700
committer Steven Thomas <steventhomas@google.com> Tue May 02 17:39:39 2017 +0000
tree a472f115541e25d5ec35d52793d406cc641bf829
parent 9f5b6aaf73ce88f8ca751fcb236b9a9e8f220d78

Avoid crash when setting buffer on deleted layer

When setLayerBuffer() was called on a layer previously destroyed by
destroyLayer() the code would crash. Instead, return an error.

Bug: 37159844

Test: Ran vr flinger in a way that would trigger the crash, and
confirmed that I now get error logs instead of crashing. Unfortunately
the error code is consumed by the Composer wrapper class and not
propagated back to the caller, but that's a separate issue (b/37901601).

Change-Id: I75a5b954d47a1deac44d03851f60f347085eca89

graphics/composer/2.1/default/ComposerClient.cpp

diff --git a/graphics/composer/2.1/default/ComposerClient.cpp b/graphics/composer/2.1/default/ComposerClient.cpp
index 87e4d3b..ce4e17f 100644
--- a/graphics/composer/2.1/default/ComposerClient.cpp
+++ b/graphics/composer/2.1/default/ComposerClient.cpp
@@ -655,18 +655,24 @@
     auto fence = readFence();
     auto dataspace = readSigned();
     auto damage = readRegion((length - 4) / 4);
+    bool closeFence = true;
 
     auto err = lookupBuffer(BufferCache::CLIENT_TARGETS,
             slot, useCache, clientTarget, &clientTarget);
     if (err == Error::NONE) {
         err = mHal.setClientTarget(mDisplay, clientTarget, fence,
                 dataspace, damage);
-
-        updateBuffer(BufferCache::CLIENT_TARGETS, slot, useCache,
-                clientTarget);
+        auto updateBufErr = updateBuffer(BufferCache::CLIENT_TARGETS, slot,
+                useCache, clientTarget);
+        if (err == Error::NONE) {
+            closeFence = false;
+            err = updateBufErr;
+        }
+    }
+    if (closeFence) {
+        close(fence);
     }
     if (err != Error::NONE) {
-        close(fence);
         mWriter.setError(getCommandLoc(), err);
     }
 
@@ -683,17 +689,23 @@
     auto slot = read();
     auto outputBuffer = readHandle(&useCache);
     auto fence = readFence();
+    bool closeFence = true;
 
     auto err = lookupBuffer(BufferCache::OUTPUT_BUFFERS,
             slot, useCache, outputBuffer, &outputBuffer);
     if (err == Error::NONE) {
         err = mHal.setOutputBuffer(mDisplay, outputBuffer, fence);
-
-        updateBuffer(BufferCache::OUTPUT_BUFFERS,
-            slot, useCache, outputBuffer);
+        auto updateBufErr = updateBuffer(BufferCache::OUTPUT_BUFFERS,
+                slot, useCache, outputBuffer);
+        if (err == Error::NONE) {
+            closeFence = false;
+            err = updateBufErr;
+        }
+    }
+    if (closeFence) {
+        close(fence);
     }
     if (err != Error::NONE) {
-        close(fence);
         mWriter.setError(getCommandLoc(), err);
     }
 
@@ -786,17 +798,23 @@
     auto slot = read();
     auto buffer = readHandle(&useCache);
     auto fence = readFence();
+    bool closeFence = true;
 
     auto err = lookupBuffer(BufferCache::LAYER_BUFFERS,
             slot, useCache, buffer, &buffer);
     if (err == Error::NONE) {
         err = mHal.setLayerBuffer(mDisplay, mLayer, buffer, fence);
-
-        updateBuffer(BufferCache::LAYER_BUFFERS,
-            slot, useCache, buffer);
+        auto updateBufErr = updateBuffer(BufferCache::LAYER_BUFFERS, slot,
+                useCache, buffer);
+        if (err == Error::NONE) {
+            closeFence = false;
+            err = updateBufErr;
+        }
+    }
+    if (closeFence) {
+        close(fence);
     }
     if (err != Error::NONE) {
-        close(fence);
         mWriter.setError(getCommandLoc(), err);
     }
 
@@ -915,8 +933,10 @@
     auto err = lookupLayerSidebandStream(stream, &stream);
     if (err == Error::NONE) {
         err = mHal.setLayerSidebandStream(mDisplay, mLayer, stream);
-
-        updateLayerSidebandStream(stream);
+        auto updateErr = updateLayerSidebandStream(stream);
+        if (err == Error::NONE) {
+            err = updateErr;
+        }
     }
     if (err != Error::NONE) {
         mWriter.setError(getCommandLoc(), err);
@@ -1097,21 +1117,24 @@
     return Error::NONE;
 }
 
-void ComposerClient::CommandReader::updateBuffer(BufferCache cache,
+Error ComposerClient::CommandReader::updateBuffer(BufferCache cache,
         uint32_t slot, bool useCache, buffer_handle_t handle)
 {
     // handle was looked up from cache
     if (useCache) {
-        return;
+        return Error::NONE;
     }
 
     std::lock_guard<std::mutex> lock(mClient.mDisplayDataMutex);
 
     BufferCacheEntry* entry = nullptr;
-    lookupBufferCacheEntryLocked(cache, slot, &entry);
-    LOG_FATAL_IF(!entry, "failed to find the cache entry to update");
+    Error error = lookupBufferCacheEntryLocked(cache, slot, &entry);
+    if (error != Error::NONE) {
+      return error;
+    }
 
     *entry = handle;
+    return Error::NONE;
 }
 
 } // namespace implementation