SurfaceFlinger: Safely cast from IBinder to Layer::Handle

Bug: b/193034677, b/193034683, b/193033243
Test: go/wm-smoke, presubmit

Change-Id: Iece64fca254edfd0b82e05ad9629824b2364cc13
Merged-In: Iece64fca254edfd0b82e05ad9629824b2364cc13

6 files changed, 49 insertions, 81 deletions

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index e4a777f3c7..6ee13ce508 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -819,11 +819,7 @@ void Layer::setZOrderRelativeOf(const wp<Layer>& relativeOf) {
 }
 
 bool Layer::setRelativeLayer(const sp<IBinder>& relativeToHandle, int32_t relativeZ) {
-    sp<Handle> handle = static_cast<Handle*>(relativeToHandle.get());
-    if (handle == nullptr) {
-        return false;
-    }
-    sp<Layer> relative = handle->owner.promote();
+    sp<Layer> relative = fromHandle(relativeToHandle).promote();
     if (relative == nullptr) {
         return false;
     }
@@ -1609,8 +1605,7 @@ void Layer::setChildrenDrawingParent(const sp<Layer>& newParent) {
 bool Layer::reparent(const sp<IBinder>& newParentHandle) {
     sp<Layer> newParent;
     if (newParentHandle != nullptr) {
-        auto handle = static_cast<Handle*>(newParentHandle.get());
-        newParent = handle->owner.promote();
+        newParent = fromHandle(newParentHandle).promote();
         if (newParent == nullptr) {
             ALOGE("Unable to promote Layer handle");
             return false;
@@ -1985,24 +1980,10 @@ void Layer::commitChildList() {
     mDrawingParent = mCurrentParent;
 }
 
-static wp<Layer> extractLayerFromBinder(const wp<IBinder>& weakBinderHandle) {
-    if (weakBinderHandle == nullptr) {
-        return nullptr;
-    }
-    sp<IBinder> binderHandle = weakBinderHandle.promote();
-    if (binderHandle == nullptr) {
-        return nullptr;
-    }
-    sp<Layer::Handle> handle = static_cast<Layer::Handle*>(binderHandle.get());
-    if (handle == nullptr) {
-        return nullptr;
-    }
-    return handle->owner;
-}
 
 void Layer::setInputInfo(const InputWindowInfo& info) {
     mDrawingState.inputInfo = info;
-    mDrawingState.touchableRegionCrop = extractLayerFromBinder(info.touchableRegionCropHandle);
+    mDrawingState.touchableRegionCrop = fromHandle(info.touchableRegionCropHandle.promote());
     mDrawingState.modified = true;
     mFlinger->mInputInfoChanged = true;
     setTransactionFlags(eTransactionNeeded);
@@ -2561,6 +2542,23 @@ void Layer::setClonedChild(const sp<Layer>& clonedChild) {
     mFlinger->mNumClones++;
 }
 
+const String16 Layer::Handle::kDescriptor = String16("android.Layer.Handle");
+
+wp<Layer> Layer::fromHandle(const sp<IBinder>& handleBinder) {
+    if (handleBinder == nullptr) {
+        return nullptr;
+    }
+
+    BBinder* b = handleBinder->localBinder();
+    if (b == nullptr || b->getInterfaceDescriptor() != Handle::kDescriptor) {
+        return nullptr;
+    }
+
+    // We can safely cast this binder since its local and we verified its interface descriptor.
+    sp<Handle> handle = static_cast<Handle*>(handleBinder.get());
+    return handle->owner;
+}
+
 // ---------------------------------------------------------------------------
 
 std::ostream& operator<<(std::ostream& stream, const Layer::FrameRate& rate) {
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 59f5b0dc73..8905548b64 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -289,16 +289,17 @@ public:
     class LayerCleaner {
         sp<SurfaceFlinger> mFlinger;
         sp<Layer> mLayer;
+        BBinder* mHandle;
 
     protected:
         ~LayerCleaner() {
             // destroy client resources
-            mFlinger->onHandleDestroyed(mLayer);
+            mFlinger->onHandleDestroyed(mHandle, mLayer);
         }
 
     public:
-        LayerCleaner(const sp<SurfaceFlinger>& flinger, const sp<Layer>& layer)
-              : mFlinger(flinger), mLayer(layer) {}
+        LayerCleaner(const sp<SurfaceFlinger>& flinger, const sp<Layer>& layer, BBinder* handle)
+              : mFlinger(flinger), mLayer(layer), mHandle(handle) {}
     };
 
     /*
@@ -312,11 +313,15 @@ public:
     class Handle : public BBinder, public LayerCleaner {
     public:
         Handle(const sp<SurfaceFlinger>& flinger, const sp<Layer>& layer)
-              : LayerCleaner(flinger, layer), owner(layer) {}
+              : LayerCleaner(flinger, layer, this), owner(layer) {}
+        const String16& getInterfaceDescriptor() const override { return kDescriptor; }
 
+        static const String16 kDescriptor;
         wp<Layer> owner;
     };
 
+    static wp<Layer> fromHandle(const sp<IBinder>& handle);
+
     explicit Layer(const LayerCreationArgs& args);
     virtual ~Layer();
 
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 230810c936..fe43a2091d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3611,7 +3611,7 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(
 
         sp<Layer> layer = nullptr;
         if (s.surface) {
-            layer = fromHandleLocked(s.surface).promote();
+            layer = fromHandle(s.surface).promote();
         } else if (s.hasBufferChanges()) {
             ALOGW("Transaction with buffer, but no Layer?");
             continue;
@@ -3778,7 +3778,7 @@ void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin
                 setClientStateLocked(frameTimelineInfo, state, desiredPresentTime, isAutoTimestamp,
                                      postTime, permissions, listenerCallbacksWithSurfaces);
         if ((flags & eAnimation) && state.state.surface) {
-            if (const auto layer = fromHandleLocked(state.state.surface).promote(); layer) {
+            if (const auto layer = fromHandle(state.state.surface).promote(); layer) {
                 mScheduler->recordLayerHistory(layer.get(),
                                                isAutoTimestamp ? 0 : desiredPresentTime,
                                                LayerHistory::LayerUpdateType::AnimationTX);
@@ -3933,13 +3933,11 @@ uint32_t SurfaceFlinger::setClientStateLocked(
         if (what & layer_state_t::eLayerCreated) {
             layer = handleLayerCreatedLocked(s.surface);
             if (layer) {
-                // put the created layer into mLayersByLocalBinderToken.
-                mLayersByLocalBinderToken.emplace(s.surface->localBinder(), layer);
                 flags |= eTransactionNeeded | eTraversalNeeded;
                 mLayersAdded = true;
             }
         } else {
-            layer = fromHandleLocked(s.surface).promote();
+            layer = fromHandle(s.surface).promote();
         }
     } else {
         // The client may provide us a null handle. Treat it as if the layer was removed.
@@ -4267,7 +4265,7 @@ status_t SurfaceFlinger::mirrorLayer(const sp<Client>& client, const sp<IBinder>
 
     {
         Mutex::Autolock _l(mStateLock);
-        mirrorFrom = fromHandleLocked(mirrorFromHandle).promote();
+        mirrorFrom = fromHandle(mirrorFromHandle).promote();
         if (!mirrorFrom) {
             return NAME_NOT_FOUND;
         }
@@ -4465,7 +4463,7 @@ void SurfaceFlinger::markLayerPendingRemovalLocked(const sp<Layer>& layer) {
     setTransactionFlags(eTransactionNeeded);
 }
 
-void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer) {
+void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer) {
     Mutex::Autolock lock(mStateLock);
     // If a layer has a parent, we allow it to out-live it's handle
     // with the idea that the parent holds a reference and will eventually
@@ -4476,17 +4474,7 @@ void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer) {
         mCurrentState.layersSortedByZ.remove(layer);
     }
     markLayerPendingRemovalLocked(layer);
-
-    auto it = mLayersByLocalBinderToken.begin();
-    while (it != mLayersByLocalBinderToken.end()) {
-        if (it->second == layer) {
-            mBufferCountTracker.remove(it->first->localBinder());
-            it = mLayersByLocalBinderToken.erase(it);
-        } else {
-            it++;
-        }
-    }
-
+    mBufferCountTracker.remove(handle);
     layer.clear();
 }
 
@@ -6089,7 +6077,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args,
     {
         Mutex::Autolock lock(mStateLock);
 
-        parent = fromHandleLocked(args.layerHandle).promote();
+        parent = fromHandle(args.layerHandle).promote();
         if (parent == nullptr || parent->isRemovedFromCurrentState()) {
             ALOGE("captureLayers called with an invalid or removed parent");
             return NAME_NOT_FOUND;
@@ -6120,7 +6108,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args,
         reqSize = ui::Size(crop.width() * args.frameScaleX, crop.height() * args.frameScaleY);
 
         for (const auto& handle : args.excludeHandles) {
-            sp<Layer> excludeLayer = fromHandleLocked(handle).promote();
+            sp<Layer> excludeLayer = fromHandle(handle).promote();
             if (excludeLayer != nullptr) {
                 excludeLayers.emplace(excludeLayer);
             } else {
@@ -6616,24 +6604,8 @@ status_t SurfaceFlinger::getDesiredDisplayModeSpecs(const sp<IBinder>& displayTo
     }
 }
 
-wp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) {
-    Mutex::Autolock _l(mStateLock);
-    return fromHandleLocked(handle);
-}
-
-wp<Layer> SurfaceFlinger::fromHandleLocked(const sp<IBinder>& handle) const {
-    BBinder* b = nullptr;
-    if (handle) {
-        b = handle->localBinder();
-    }
-    if (b == nullptr) {
-        return nullptr;
-    }
-    auto it = mLayersByLocalBinderToken.find(b);
-    if (it != mLayersByLocalBinderToken.end()) {
-        return it->second;
-    }
-    return nullptr;
+wp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) const {
+    return Layer::fromHandle(handle);
 }
 
 void SurfaceFlinger::onLayerFirstRef(Layer* layer) {
@@ -6938,7 +6910,7 @@ sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle) {
     sp<Layer> parent;
     bool allowAddRoot = state->addToRoot;
     if (state->initialParent != nullptr) {
-        parent = fromHandleLocked(state->initialParent.promote()).promote();
+        parent = fromHandle(state->initialParent.promote()).promote();
         if (parent == nullptr) {
             ALOGE("Invalid parent %p", state->initialParent.unsafe_get());
             allowAddRoot = false;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 380f444221..c3d837e86c 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -327,8 +327,7 @@ public:
     // Returns nullptr if the handle does not point to an existing layer.
     // Otherwise, returns a weak reference so that callers off the main-thread
     // won't accidentally hold onto the last strong reference.
-    wp<Layer> fromHandle(const sp<IBinder>& handle);
-    wp<Layer> fromHandleLocked(const sp<IBinder>& handle) const REQUIRES(mStateLock);
+    wp<Layer> fromHandle(const sp<IBinder>& handle) const;
 
     // If set, disables reusing client composition buffers. This can be set by
     // debug.sf.disable_client_composition_cache
@@ -898,7 +897,7 @@ private:
     // called when all clients have released all their references to
     // this layer meaning it is entirely safe to destroy all
     // resources associated to this layer.
-    void onHandleDestroyed(sp<Layer>& layer);
+    void onHandleDestroyed(BBinder* handle, sp<Layer>& layer);
     void markLayerPendingRemovalLocked(const sp<Layer>& layer);
 
     // add a layer to SurfaceFlinger
@@ -1290,8 +1289,6 @@ private:
         std::optional<DisplayIdGenerator<HalVirtualDisplayId>> hal;
     } mVirtualDisplayIdGenerators;
 
-    std::unordered_map<BBinder*, wp<Layer>> mLayersByLocalBinderToken GUARDED_BY(mStateLock);
-
     // don't use a lock for these, we don't care
     int mDebugRegion = 0;
     bool mDebugDisableHWC = false;
diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp
index 23ab7c8bab..837b2e8924 100644
--- a/services/surfaceflinger/SurfaceInterceptor.cpp
+++ b/services/surfaceflinger/SurfaceInterceptor.cpp
@@ -183,12 +183,9 @@ status_t SurfaceInterceptor::writeProtoFileLocked() {
     return NO_ERROR;
 }
 
-const sp<const Layer> SurfaceInterceptor::getLayer(const wp<const IBinder>& weakHandle) const {
-    const sp<const IBinder>& handle(weakHandle.promote());
-    const auto layerHandle(static_cast<const Layer::Handle*>(handle.get()));
-    const sp<const Layer> layer(layerHandle->owner.promote());
-    // layer could be a nullptr at this point
-    return layer;
+const sp<const Layer> SurfaceInterceptor::getLayer(const wp<IBinder>& weakHandle) const {
+    sp<IBinder> handle = weakHandle.promote();
+    return Layer::fromHandle(handle).promote();
 }
 
 int32_t SurfaceInterceptor::getLayerId(const sp<const Layer>& layer) const {
@@ -203,12 +200,11 @@ int32_t SurfaceInterceptor::getLayerIdFromWeakRef(const wp<const Layer>& layer)
     return strongLayer == nullptr ? -1 : getLayerId(strongLayer);
 }
 
-int32_t SurfaceInterceptor::getLayerIdFromHandle(const sp<const IBinder>& handle) const {
+int32_t SurfaceInterceptor::getLayerIdFromHandle(const sp<IBinder>& handle) const {
     if (handle == nullptr) {
         return -1;
     }
-    const auto layerHandle(static_cast<const Layer::Handle*>(handle.get()));
-    const sp<const Layer> layer(layerHandle->owner.promote());
+    const sp<const Layer> layer = Layer::fromHandle(handle).promote();
     return layer == nullptr ? -1 : getLayerId(layer);
 }
 
diff --git a/services/surfaceflinger/SurfaceInterceptor.h b/services/surfaceflinger/SurfaceInterceptor.h
index 673f9e789d..c9555969dc 100644
--- a/services/surfaceflinger/SurfaceInterceptor.h
+++ b/services/surfaceflinger/SurfaceInterceptor.h
@@ -133,10 +133,10 @@ private:
     void addInitialDisplayStateLocked(Increment* increment, const DisplayDeviceState& display);
 
     status_t writeProtoFileLocked();
-    const sp<const Layer> getLayer(const wp<const IBinder>& weakHandle) const;
+    const sp<const Layer> getLayer(const wp<IBinder>& weakHandle) const;
     int32_t getLayerId(const sp<const Layer>& layer) const;
     int32_t getLayerIdFromWeakRef(const wp<const Layer>& layer) const;
-    int32_t getLayerIdFromHandle(const sp<const IBinder>& weakHandle) const;
+    int32_t getLayerIdFromHandle(const sp<IBinder>& weakHandle) const;
 
     Increment* createTraceIncrementLocked();
     void addSurfaceCreationLocked(Increment* increment, const sp<const Layer>& layer);