commit 88b85e1f9b2fb3cf1ab9da1967e1d454be2d5be0
author Robert Carr <racarr@google.com> Mon Mar 21 15:47:35 2022 -0700
committer Rob Carr <racarr@google.com> Tue Mar 22 22:56:25 2022 +0000
tree 982ecba4a76554b8bb3af9854bd3025f8973357b
parent f4b49765605223dba7b804c351a98a3c6bb676f9

SurfaceFlinger: Detect Rel Z loops

Telemetry indicates we are sometimes seeing crashes
due to infinite recursion in isHiddenByPolicy. We detect this
condition (relative Z loop) and add a helpful log. The performance
impact should be minimal as relative Z is rare, and
the traversal isn't doing any heavy work or sp<> promotion.

Bug: 213420016
Bug: 195298154
Bug: 219774897
Test: Existing tests pass
Change-Id: Ia11854654e99288c5846fe7c62ebabf37daeb139

services/surfaceflinger/Layer.cpp

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index aeaf1e1..8159150 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -833,6 +833,14 @@
         return false;
     }
 
+    if (CC_UNLIKELY(relative->usingRelativeZ(LayerVector::StateSet::Drawing)) &&
+        (relative->mDrawingState.zOrderRelativeOf == this)) {
+        ALOGE("Detected relative layer loop between %s and %s",
+              mName.c_str(), relative->mName.c_str());
+        ALOGE("Ignoring new call to set relative layer");
+        return false;
+    }
+
     mFlinger->mSomeChildrenChanged = true;
 
     mDrawingState.sequence++;
@@ -1990,6 +1998,18 @@
     }
 }
 
+bool Layer::findInHierarchy(const sp<Layer>& l) {
+    if (l == this) {
+        return true;
+    }
+    for (auto& child : mDrawingChildren) {
+      if (child->findInHierarchy(l)) {
+          return true;
+      }
+    }
+    return false;
+}
+
 void Layer::commitChildList() {
     for (size_t i = 0; i < mCurrentChildren.size(); i++) {
         const auto& child = mCurrentChildren[i];
@@ -1997,6 +2017,17 @@
     }
     mDrawingChildren = mCurrentChildren;
     mDrawingParent = mCurrentParent;
+    if (CC_UNLIKELY(usingRelativeZ(LayerVector::StateSet::Drawing))) {
+        auto zOrderRelativeOf = mDrawingState.zOrderRelativeOf.promote();
+        if (zOrderRelativeOf == nullptr) return;
+        if (findInHierarchy(zOrderRelativeOf)) {
+            ALOGE("Detected Z ordering loop between %s and %s", mName.c_str(),
+                  zOrderRelativeOf->mName.c_str());
+            ALOGE("Severing rel Z loop, potentially dangerous");
+            mDrawingState.isRelativeOf = false;
+            zOrderRelativeOf->removeZOrderRelative(this);
+        }
+    }
 }