commit c0df312c34d92161e955fd1057afaa1337b7a56d
author Robert Carr <racarr@google.com> Thu Apr 11 13:18:21 2019 -0700
committer Robert Carr <racarr@google.com> Mon Apr 15 14:30:19 2019 -0700
tree a7fffbb585f25f1a377f78a97f80c423593dc666
parent 27e6f2b579ebca586411ced6106c48b014730160

SurfaceFlinger: Validate layers before casting.

Reinterpret casting random IBinder = no-fun. I first attempted
to use inheritance of "getInterfaceDescriptor" in Layer::Handle but
departing from "standard-layout" (e.g. using virtual methods) means that
downcasting with static/reinterpret_cast is no longer valid. Instead I opted
for the pattern the system-server uses of maintaing a map.

Now that we look up the handle in a map rather than casting IBinder
to Layer::Handle we need to make sure we have unique instances of the
handle. In general this is true but we weren't doing this in the
createWithSurfaceParent where we had an extra call to getHandle. Here
we both refactor createWithSurfaceParent so it works with the new
changes and also add protection for getHandle. We also fix an error
where the handle map was populated outside of lock.

Bug: 129768960
Test: InvalidHandles_test.cpp ASurfaceControlTest SurfaceControlTest
Change-Id: I869bf6164c8d8203af7486ed1b12a763d5a56662

services/surfaceflinger/SurfaceFlinger.h

diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 7a7ad33..626140f 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -313,6 +313,8 @@
         return mTransactionCompletedThread;
     }
 
+    sp<Layer> fromHandle(const sp<IBinder>& handle) REQUIRES(mStateLock);
+
 private:
     friend class BufferLayer;
     friend class BufferQueueLayer;
@@ -575,7 +577,8 @@
      */
     status_t createLayer(const String8& name, const sp<Client>& client, uint32_t w, uint32_t h,
                          PixelFormat format, uint32_t flags, LayerMetadata metadata,
-                         sp<IBinder>* handle, sp<IGraphicBufferProducer>* gbp, sp<Layer>* parent);
+                         sp<IBinder>* handle, sp<IGraphicBufferProducer>* gbp,
+                         const sp<IBinder>& parentHandle, const sp<Layer>& parentLayer = nullptr);
 
     status_t createBufferQueueLayer(const sp<Client>& client, const String8& name, uint32_t w,
                                     uint32_t h, uint32_t flags, LayerMetadata metadata,
@@ -603,12 +606,10 @@
     void markLayerPendingRemovalLocked(const sp<Layer>& layer);
 
     // add a layer to SurfaceFlinger
-    status_t addClientLayer(const sp<Client>& client,
-            const sp<IBinder>& handle,
-            const sp<IGraphicBufferProducer>& gbc,
-            const sp<Layer>& lbc,
-            const sp<Layer>& parent,
-            bool addToCurrentState);
+    status_t addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,
+                            const sp<IGraphicBufferProducer>& gbc, const sp<Layer>& lbc,
+                            const sp<IBinder>& parentHandle, const sp<Layer>& parentLayer,
+                            bool addToCurrentState);
 
     // Traverse through all the layers and compute and cache its bounds.
     void computeLayerBounds();
@@ -981,6 +982,9 @@
     std::map<wp<IBinder>, sp<DisplayDevice>> mDisplays;
     std::unordered_map<DisplayId, sp<IBinder>> mPhysicalDisplayTokens;
 
+    // protected by mStateLock
+    std::unordered_map<BBinder*, wp<Layer>> mLayersByLocalBinderToken;
+
     // don't use a lock for these, we don't care
     int mDebugRegion = 0;
     bool mDebugDisableHWC = false;