commit 20261cb526480d8a98f4c393ac0f28cf0a250892
author Chia-I Wu <olv@google.com> Thu Aug 23 12:55:44 2018 -0700
committer Chia-I Wu <olv@google.com> Mon Aug 27 11:20:24 2018 -0700
tree c874c06d6401fb03606a42fc49138b52e8ac193c
parent c80dcbb574106f3ff126d6b77cbcd12463089301

surfaceflinger: fix race conditions in captureScreen

The display was looked up without holding the state lock.  It was
also used (including indirectly from updateDimensions) without
holding the lock either.

Inline updateDimensions in captureScreen with the state lock held.
Stop calling updateDimensions in captureLayers.

Bug: 113041375
Test: take screenshot, rotate screen, screencap
Change-Id: I8b361847c44373ce08930d906ec4a995efd1c21b

services/surfaceflinger/SurfaceFlinger.cpp

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2dc55c6..5acfe59 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4919,16 +4919,44 @@
 
     auto renderAreaRotation = fromSurfaceComposerRotation(rotation);
 
-    const auto display = getDisplayDeviceLocked(displayToken);
-    if (!display) return BAD_VALUE;
+    sp<DisplayDevice> display;
+    {
+        Mutex::Autolock _l(mStateLock);
 
-    const Rect& dispScissor = display->getScissor();
-    if (!dispScissor.isEmpty()) {
-        sourceCrop.set(dispScissor);
-        // adb shell screencap will default reqWidth and reqHeight to zeros.
-        if (reqWidth == 0 || reqHeight == 0) {
-            reqWidth = uint32_t(display->getViewport().width());
-            reqHeight = uint32_t(display->getViewport().height());
+        display = getDisplayDeviceLocked(displayToken);
+        if (!display) return BAD_VALUE;
+
+        const Rect& dispScissor = display->getScissor();
+        if (!dispScissor.isEmpty()) {
+            sourceCrop.set(dispScissor);
+            // adb shell screencap will default reqWidth and reqHeight to zeros.
+            if (reqWidth == 0 || reqHeight == 0) {
+                reqWidth = uint32_t(display->getViewport().width());
+                reqHeight = uint32_t(display->getViewport().height());
+            }
+        }
+
+        // get screen geometry
+        uint32_t width = display->getWidth();
+        uint32_t height = display->getHeight();
+
+        if (renderAreaRotation & ui::Transform::ROT_90) {
+            std::swap(width, height);
+        }
+
+        if (mPrimaryDisplayOrientation & DisplayState::eOrientationSwapMask) {
+            std::swap(width, height);
+        }
+
+        if ((reqWidth > width) || (reqHeight > height)) {
+            ALOGE("size mismatch (%d, %d) > (%d, %d)", reqWidth, reqHeight, width, height);
+        } else {
+            if (reqWidth == 0) {
+                reqWidth = width;
+            }
+            if (reqHeight == 0) {
+                reqHeight = height;
+            }
         }
     }
 
@@ -5040,6 +5068,14 @@
     int32_t reqWidth = crop.width() * frameScale;
     int32_t reqHeight = crop.height() * frameScale;
 
+    // really small crop or frameScale
+    if (reqWidth <= 0) {
+        reqWidth = 1;
+    }
+    if (reqHeight <= 0) {
+        reqHeight = 1;
+    }
+
     LayerRenderArea renderArea(this, parent, crop, reqWidth, reqHeight, childrenOnly);
 
     auto traverseLayers = [parent, childrenOnly](const LayerVector::Visitor& visitor) {
@@ -5061,8 +5097,6 @@
                                              bool useIdentityTransform) {
     ATRACE_CALL();
 
-    renderArea.updateDimensions(mPrimaryDisplayOrientation);
-
     const uint32_t usage = GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN |
             GRALLOC_USAGE_HW_RENDER | GRALLOC_USAGE_HW_TEXTURE;
     *outBuffer = new GraphicBuffer(renderArea.getReqWidth(), renderArea.getReqHeight(),