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
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index e4a777f..6ee13ce 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -819,11 +819,7 @@
}
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 @@
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 @@
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 @@
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 59f5b0d..8905548 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -289,16 +289,17 @@
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 @@
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 230810c..fe43a20 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3611,7 +3611,7 @@
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 @@
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 @@
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 @@
{
Mutex::Autolock _l(mStateLock);
- mirrorFrom = fromHandleLocked(mirrorFromHandle).promote();
+ mirrorFrom = fromHandle(mirrorFromHandle).promote();
if (!mirrorFrom) {
return NAME_NOT_FOUND;
}
@@ -4465,7 +4463,7 @@
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 @@
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 @@
{
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 @@
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 @@
}
}
-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> 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 380f444..c3d837e 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -327,8 +327,7 @@
// 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 @@
// 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 @@
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 23ab7c8..837b2e8 100644
--- a/services/surfaceflinger/SurfaceInterceptor.cpp
+++ b/services/surfaceflinger/SurfaceInterceptor.cpp
@@ -183,12 +183,9 @@
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 @@
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 673f9e7..c955596 100644
--- a/services/surfaceflinger/SurfaceInterceptor.h
+++ b/services/surfaceflinger/SurfaceInterceptor.h
@@ -133,10 +133,10 @@
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);
