SurfaceFlinger: Fixes around rotation and cropping.

SurfaceFlinger's renderer is not prepared to handle cropping in the face of
arbitrary rotation. To see the problem observe that if we have a square parent,
and a child of the same size, then we rotate the child 45 degrees around it's
center, the child must now be cropped to a non rectangular 8 sided region.

We can fix this problem in the future (b/109894387), but for now we are lucky.
SurfaceControl is private API, and the WindowManager only uses rotation in one
case, which is on a top level layer in which cropping need not be an issue
(this case is the screen rotation animation, where all the windows are rotated
together).

However given that the abuse of rotation matrices could lead to surfaces
extending outside of theire intended crop, we need to prevent non root-clients
without permission ACCESS_SURFACE_FLINGER (a.k.a. everyone except WindowManager
and tests) from setting non rectangle preserving transformations.

Our sad story continues, with the implementation of computeBounds. Notice the
intersection with the parent window is done in screen space by applying
and then inverting the transformation. However since the transformation doesn't
preserve rectangles, we get a different, in-correct, and larger result
from applying and inverting the transformation.

We don't need to be performing this computation in screen space, it's enough to
apply the local transform relative to the parent and then intersect with the
parent's computed bounds in the parent space. When we write the logic this way
it means we will only produce incorrect results for children who rotate outside
of their visible region. In the case of the WindowManager rotation animation
it rotates top level layers which do not have parents, and so we will
not produce incorrect results. We lock down other cases and clients
as described above.

Unfortunately our story continues, since our implementation of final crop was relying
on transforming Layers up to screen-space, this will no longer work with the
new implementation of compute bounds. We have to change setFinalCrop to crop
in parent-space rather than the final screen space. This is a semantic change, but
luckily there is only one user of setFinalCrop and it is on a layer whose parent
(The WM animation Layer) is already in screen space, so it's not a semantic change
for any actual clients.

Test: Manual.
Bug: 69913240
Bug: 109894387
Change-Id: I522e258cee03ac8e3609a40f53461119b7c45532

3 files changed, 55 insertions, 17 deletions

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 2595ec1a05..6d5a598e89 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -345,20 +345,25 @@ FloatRect Layer::computeBounds(const Region& activeTransparentRegion) const {
         win.intersect(s.crop, &win);
     }
 
-    Rect bounds = win;
     const auto& p = mDrawingParent.promote();
+    FloatRect floatWin = win.toFloatRect();
+    FloatRect parentBounds = floatWin;
     if (p != nullptr) {
-        // Look in computeScreenBounds recursive call for explanation of
-        // why we pass false here.
-        bounds = p->computeScreenBounds(false /* reduceTransparentRegion */);
+        // We pass an empty Region here for reasons mirroring that of the case described in
+        // the computeScreenBounds reduceTransparentRegion=false case.
+        parentBounds = p->computeBounds(Region());
     }
 
-    Transform t = getTransform();
+    Transform t = s.active.transform;
 
-    FloatRect floatWin = win.toFloatRect();
-    if (p != nullptr) {
+
+    if (p != nullptr || !s.finalCrop.isEmpty()) {
         floatWin = t.transform(floatWin);
-        floatWin = floatWin.intersect(bounds.toFloatRect());
+        floatWin = floatWin.intersect(parentBounds);
+
+        if (!s.finalCrop.isEmpty()) {
+            floatWin = floatWin.intersect(s.finalCrop.toFloatRect());
+        }
         floatWin = t.inverse().transform(floatWin);
     }
 
@@ -1249,7 +1254,15 @@ bool Layer::setColor(const half3& color) {
     return true;
 }
 
-bool Layer::setMatrix(const layer_state_t::matrix22_t& matrix) {
+bool Layer::setMatrix(const layer_state_t::matrix22_t& matrix,
+        bool allowNonRectPreservingTransforms) {
+    Transform t;
+    t.set(matrix.dsdx, matrix.dtdy, matrix.dtdx, matrix.dsdy);
+
+    if (!allowNonRectPreservingTransforms && !t.preserveRects()) {
+        ALOGW("Attempt to set rotation matrix without permission ACCESS_SURFACE_FLINGER ignored");
+        return false;
+    }
     mCurrentState.sequence++;
     mCurrentState.requested.transform.set(matrix.dsdx, matrix.dtdy, matrix.dtdx, matrix.dsdy);
     mCurrentState.modified = true;
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 34811fb152..f4ae21aad5 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -258,7 +258,7 @@ public:
     // Set a 2x2 transformation matrix on the layer. This transform
     // will be applied after parent transforms, but before any final
     // producer specified transform.
-    bool setMatrix(const layer_state_t::matrix22_t& matrix);
+    bool setMatrix(const layer_state_t::matrix22_t& matrix, bool allowNonRectPreservingTransforms);
 
     // This second set of geometry attributes are controlled by
     // setGeometryAppliesWithResize, and their default mode is to be
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 87baf8c8a8..eae607f549 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3327,6 +3327,18 @@ uint32_t SurfaceFlinger::setDisplayStateLocked(const DisplayState& s)
     return flags;
 }
 
+bool callingThreadHasUnscopedSurfaceFlingerAccess() {
+    IPCThreadState* ipc = IPCThreadState::self();
+    const int pid = ipc->getCallingPid();
+    const int uid = ipc->getCallingUid();
+
+    if ((uid != AID_GRAPHICS && uid != AID_SYSTEM) &&
+            !PermissionCache::checkPermission(sAccessSurfaceFlinger, pid, uid)) {
+        return false;
+    }
+    return true;
+}
+
 uint32_t SurfaceFlinger::setClientStateLocked(const ComposerState& composerState) {
     const layer_state_t& s = composerState.state;
     sp<Client> client(static_cast<Client*>(composerState.client.get()));
@@ -3408,7 +3420,22 @@ uint32_t SurfaceFlinger::setClientStateLocked(const ComposerState& composerState
             flags |= eTraversalNeeded;
     }
     if (what & layer_state_t::eMatrixChanged) {
-        if (layer->setMatrix(s.matrix))
+        // TODO: b/109894387
+        //
+        // SurfaceFlinger's renderer is not prepared to handle cropping in the face of arbitrary
+        // rotation. To see the problem observe that if we have a square parent, and a child
+        // of the same size, then we rotate the child 45 degrees around it's center, the child
+        // must now be cropped to a non rectangular 8 sided region.
+        //
+        // Of course we can fix this in the future. For now, we are lucky, SurfaceControl is
+        // private API, and the WindowManager only uses rotation in one case, which is on a top
+        // level layer in which cropping is not an issue.
+        //
+        // However given that abuse of rotation matrices could lead to surfaces extending outside
+        // of cropped areas, we need to prevent non-root clients without permission ACCESS_SURFACE_FLINGER
+        // (a.k.a. everyone except WindowManager and tests) from setting non rectangle preserving
+        // transformations.
+        if (layer->setMatrix(s.matrix, callingThreadHasUnscopedSurfaceFlingerAccess()))
             flags |= eTraversalNeeded;
     }
     if (what & layer_state_t::eTransparentRegionChanged) {
@@ -4412,12 +4439,10 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) {
         case INJECT_VSYNC:
         {
             // codes that require permission check
-            IPCThreadState* ipc = IPCThreadState::self();
-            const int pid = ipc->getCallingPid();
-            const int uid = ipc->getCallingUid();
-            if ((uid != AID_GRAPHICS && uid != AID_SYSTEM) &&
-                    !PermissionCache::checkPermission(sAccessSurfaceFlinger, pid, uid)) {
-                ALOGE("Permission Denial: can't access SurfaceFlinger pid=%d, uid=%d", pid, uid);
+            if (!callingThreadHasUnscopedSurfaceFlingerAccess()) {
+                IPCThreadState* ipc = IPCThreadState::self();
+                ALOGE("Permission Denial: can't access SurfaceFlinger pid=%d, uid=%d",
+                        ipc->getCallingPid(), ipc->getCallingUid());
                 return PERMISSION_DENIED;
             }
             break;