diff options
| -rw-r--r-- | libs/gui/include/gui/SurfaceControl.h | 9 | ||||
| -rw-r--r-- | services/surfaceflinger/Client.cpp | 11 | ||||
| -rw-r--r-- | services/surfaceflinger/Layer.cpp | 1 | ||||
| -rw-r--r-- | services/surfaceflinger/RefreshRateOverlay.cpp | 4 | ||||
| -rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 118 | ||||
| -rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 14 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/Android.bp | 1 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/InvalidHandles_test.cpp | 67 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/SurfaceFlinger_test.filter | 2 |
9 files changed, 65 insertions, 162 deletions
diff --git a/libs/gui/include/gui/SurfaceControl.h b/libs/gui/include/gui/SurfaceControl.h index 23bfc0256b..55efcbfb3a 100644 --- a/libs/gui/include/gui/SurfaceControl.h +++ b/libs/gui/include/gui/SurfaceControl.h @@ -84,9 +84,6 @@ public: explicit SurfaceControl(const sp<SurfaceControl>& other); - SurfaceControl(const sp<SurfaceComposerClient>& client, const sp<IBinder>& handle, - const sp<IGraphicBufferProducer>& gbp, bool owned); - private: // can't be copied SurfaceControl& operator = (SurfaceControl& rhs); @@ -95,6 +92,12 @@ private: friend class SurfaceComposerClient; friend class Surface; + SurfaceControl( + const sp<SurfaceComposerClient>& client, + const sp<IBinder>& handle, + const sp<IGraphicBufferProducer>& gbp, + bool owned); + ~SurfaceControl(); sp<Surface> generateSurfaceLocked() const; diff --git a/services/surfaceflinger/Client.cpp b/services/surfaceflinger/Client.cpp index 508be13c81..e54b4605a4 100644 --- a/services/surfaceflinger/Client.cpp +++ b/services/surfaceflinger/Client.cpp @@ -76,9 +76,18 @@ status_t Client::createSurface(const String8& name, uint32_t w, uint32_t h, Pixe uint32_t flags, const sp<IBinder>& parentHandle, LayerMetadata metadata, sp<IBinder>* handle, sp<IGraphicBufferProducer>* gbp) { + sp<Layer> parent = nullptr; + if (parentHandle != nullptr) { + auto layerHandle = reinterpret_cast<Layer::Handle*>(parentHandle.get()); + parent = layerHandle->owner.promote(); + if (parent == nullptr) { + return NAME_NOT_FOUND; + } + } + // We rely on createLayer to check permissions. return mFlinger->createLayer(name, this, w, h, format, flags, std::move(metadata), handle, gbp, - parentHandle); + &parent); } status_t Client::createWithSurfaceParent(const String8& name, uint32_t w, uint32_t h, diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 1716c17e51..7965245823 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -125,6 +125,7 @@ Layer::~Layer() { } mFrameTracker.logAndResetStats(mName); + mFlinger->onLayerDestroyed(); } diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index 240c84ed47..e70bfe4b1e 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -31,9 +31,9 @@ bool RefreshRateOverlay::createLayer() { const status_t ret = mFlinger.createLayer(String8("RefreshRateOverlay"), mClient, 0, 0, PIXEL_FORMAT_RGBA_8888, ISurfaceComposerClient::eFXSurfaceColor, - LayerMetadata(), &mIBinder, &mGbp, nullptr); + LayerMetadata(), &mIBinder, &mGbp, &mLayer); if (ret) { - ALOGE("failed to create color layer"); + ALOGE("failed to color layer"); return false; } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 530ec304ca..fccd910795 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3418,20 +3418,12 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBinder>& handle, const sp<IGraphicBufferProducer>& gbc, const sp<Layer>& lbc, - const sp<IBinder>& parentHandle, + const sp<Layer>& parent, bool addToCurrentState) { // add this layer to the current state list { Mutex::Autolock _l(mStateLock); - sp<Layer> parent; - if (parentHandle != nullptr) { - parent = fromHandle(parentHandle); - if (parent == nullptr) { - return NAME_NOT_FOUND; - } - } - if (mNumLayers >= MAX_LAYERS) { ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers, MAX_LAYERS); @@ -4042,10 +4034,10 @@ uint32_t SurfaceFlinger::addInputWindowCommands(const InputWindowCommands& input return flags; } -status_t SurfaceFlinger::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, const sp<IBinder>& parentHandle) { +status_t SurfaceFlinger::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) { if (int32_t(w|h) < 0) { ALOGE("createLayer() failed, w or h is negative (w=%d, h=%d)", int(w), int(h)); @@ -4110,14 +4102,12 @@ status_t SurfaceFlinger::createLayer( return result; } - mLayersByLocalBinderToken.emplace((*handle)->localBinder(), layer); - if (primaryDisplayOnly) { layer->setPrimaryDisplayOnly(); } bool addToCurrentState = callingThreadHasUnscopedSurfaceFlingerAccess(); - result = addClientLayer(client, *handle, *gbp, layer, parentHandle, + result = addClientLayer(client, *handle, *gbp, layer, *parent, addToCurrentState); if (result != NO_ERROR) { return result; @@ -4235,16 +4225,6 @@ void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer) mCurrentState.layersSortedByZ.remove(layer); } markLayerPendingRemovalLocked(layer); - - auto it = mLayersByLocalBinderToken.begin(); - while (it != mLayersByLocalBinderToken.end()) { - if (it->second == layer) { - it = mLayersByLocalBinderToken.erase(it); - } else { - it++; - } - } - layer.clear(); } @@ -5514,50 +5494,34 @@ status_t SurfaceFlinger::captureLayers( const bool mChildrenOnly; }; - int reqWidth = 0; - int reqHeight = 0; - sp<Layer> parent; - Rect crop(sourceCrop); - std::unordered_set<sp<Layer>, ISurfaceComposer::SpHash<Layer>> excludeLayers; + auto layerHandle = reinterpret_cast<Layer::Handle*>(layerHandleBinder.get()); + auto parent = layerHandle->owner.promote(); - { - Mutex::Autolock _l(mStateLock); - - parent = fromHandle(layerHandleBinder); - if (parent == nullptr || parent->isRemovedFromCurrentState()) { - ALOGE("captureLayers called with an invalid or removed parent"); - return NAME_NOT_FOUND; - } + if (parent == nullptr || parent->isRemovedFromCurrentState()) { + ALOGE("captureLayers called with a removed parent"); + return NAME_NOT_FOUND; + } - const int uid = IPCThreadState::self()->getCallingUid(); - const bool forSystem = uid == AID_GRAPHICS || uid == AID_SYSTEM; - if (!forSystem && parent->getCurrentState().flags & layer_state_t::eLayerSecure) { - ALOGW("Attempting to capture secure layer: PERMISSION_DENIED"); - return PERMISSION_DENIED; - } + const int uid = IPCThreadState::self()->getCallingUid(); + const bool forSystem = uid == AID_GRAPHICS || uid == AID_SYSTEM; + if (!forSystem && parent->getCurrentState().flags & layer_state_t::eLayerSecure) { + ALOGW("Attempting to capture secure layer: PERMISSION_DENIED"); + return PERMISSION_DENIED; + } - if (sourceCrop.width() <= 0) { - crop.left = 0; - crop.right = parent->getBufferSize(parent->getCurrentState()).getWidth(); - } + Rect crop(sourceCrop); + if (sourceCrop.width() <= 0) { + crop.left = 0; + crop.right = parent->getBufferSize(parent->getCurrentState()).getWidth(); + } - if (sourceCrop.height() <= 0) { - crop.top = 0; - crop.bottom = parent->getBufferSize(parent->getCurrentState()).getHeight(); - } - reqWidth = crop.width() * frameScale; - reqHeight = crop.height() * frameScale; + if (sourceCrop.height() <= 0) { + crop.top = 0; + crop.bottom = parent->getBufferSize(parent->getCurrentState()).getHeight(); + } - for (const auto& handle : excludeHandles) { - sp<Layer> excludeLayer = fromHandle(handle); - if (excludeLayer != nullptr) { - excludeLayers.emplace(excludeLayer); - } else { - ALOGW("Invalid layer handle passed as excludeLayer to captureLayers"); - return NAME_NOT_FOUND; - } - } - } // mStateLock + int32_t reqWidth = crop.width() * frameScale; + int32_t reqHeight = crop.height() * frameScale; // really small crop or frameScale if (reqWidth <= 0) { @@ -5567,6 +5531,18 @@ status_t SurfaceFlinger::captureLayers( reqHeight = 1; } + std::unordered_set<sp<Layer>, ISurfaceComposer::SpHash<Layer>> excludeLayers; + for (const auto& handle : excludeHandles) { + BBinder* local = handle->localBinder(); + if (local != nullptr) { + auto layerHandle = reinterpret_cast<Layer::Handle*>(local); + excludeLayers.emplace(layerHandle->owner.promote()); + } else { + ALOGW("Invalid layer handle passed as excludeLayer to captureLayers"); + return NAME_NOT_FOUND; + } + } + LayerRenderArea renderArea(this, parent, crop, reqWidth, reqHeight, reqDataspace, childrenOnly); auto traverseLayers = [parent, childrenOnly, &excludeLayers](const LayerVector::Visitor& visitor) { @@ -5937,18 +5913,6 @@ void SurfaceFlinger::SetInputWindowsListener::onSetInputWindowsFinished() { mFlinger->setInputWindowsFinished(); } -sp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) { - BBinder *b = handle->localBinder(); - if (b == nullptr) { - return nullptr; - } - auto it = mLayersByLocalBinderToken.find(b); - if (it != mLayersByLocalBinderToken.end()) { - return it->second.promote(); - } - return nullptr; -} - } // namespace android #if defined(__gl_h_) diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index c0c088095f..7e8e836e6a 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -313,8 +313,6 @@ public: return mTransactionCompletedThread; } - sp<Layer> fromHandle(const sp<IBinder>& handle) REQUIRES(mStateLock); - private: friend class BufferLayer; friend class BufferQueueLayer; @@ -589,10 +587,9 @@ private: /* ------------------------------------------------------------------------ * Layer management */ - 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, const sp<IBinder>& parentHandle); + 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); status_t createBufferQueueLayer(const sp<Client>& client, const String8& name, uint32_t w, uint32_t h, uint32_t flags, LayerMetadata metadata, @@ -624,7 +621,7 @@ private: const sp<IBinder>& handle, const sp<IGraphicBufferProducer>& gbc, const sp<Layer>& lbc, - const sp<IBinder>& parentHandle, + const sp<Layer>& parent, bool addToCurrentState); // Traverse through all the layers and compute and cache its bounds. @@ -997,9 +994,6 @@ private: 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; diff --git a/services/surfaceflinger/tests/Android.bp b/services/surfaceflinger/tests/Android.bp index d3c87bf00d..f121a952d8 100644 --- a/services/surfaceflinger/tests/Android.bp +++ b/services/surfaceflinger/tests/Android.bp @@ -19,7 +19,6 @@ cc_test { srcs: [ "BufferGenerator.cpp", "Credentials_test.cpp", - "InvalidHandles_test.cpp", "Stress_test.cpp", "SurfaceInterceptor_test.cpp", "Transaction_test.cpp", diff --git a/services/surfaceflinger/tests/InvalidHandles_test.cpp b/services/surfaceflinger/tests/InvalidHandles_test.cpp deleted file mode 100644 index 42d1f5aa6c..0000000000 --- a/services/surfaceflinger/tests/InvalidHandles_test.cpp +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Copyright (C) 2019 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include <binder/Binder.h> - -#include <gtest/gtest.h> - -#include <gui/ISurfaceComposer.h> -#include <gui/SurfaceComposerClient.h> -#include <private/gui/ComposerService.h> -#include <ui/Rect.h> - -namespace android { -namespace { - -class NotALayer : public BBinder {}; - -/** - * For all of these tests we make a SurfaceControl with an invalid layer handle - * and verify we aren't able to trick SurfaceFlinger. - */ -class InvalidHandleTest : public ::testing::Test { -protected: - sp<SurfaceComposerClient> mScc; - sp<SurfaceControl> mNotSc; - void SetUp() override { - mScc = new SurfaceComposerClient; - ASSERT_EQ(NO_ERROR, mScc->initCheck()); - mNotSc = makeNotSurfaceControl(); - } - - sp<SurfaceControl> makeNotSurfaceControl() { - return new SurfaceControl(mScc, new NotALayer(), nullptr, true); - } -}; - -TEST_F(InvalidHandleTest, createSurfaceInvalidHandle) { - auto notSc = makeNotSurfaceControl(); - ASSERT_EQ(nullptr, - mScc->createSurface(String8("lolcats"), 19, 47, PIXEL_FORMAT_RGBA_8888, 0, - notSc.get()) - .get()); -} - -TEST_F(InvalidHandleTest, captureLayersInvalidHandle) { - sp<ISurfaceComposer> sf(ComposerService::getComposerService()); - sp<GraphicBuffer> outBuffer; - - ASSERT_EQ(NAME_NOT_FOUND, - sf->captureLayers(mNotSc->getHandle(), &outBuffer, Rect::EMPTY_RECT, 1.0f)); -} - -} // namespace -} // namespace android diff --git a/services/surfaceflinger/tests/SurfaceFlinger_test.filter b/services/surfaceflinger/tests/SurfaceFlinger_test.filter index a8d09eae6a..be862c9d16 100644 --- a/services/surfaceflinger/tests/SurfaceFlinger_test.filter +++ b/services/surfaceflinger/tests/SurfaceFlinger_test.filter @@ -1,5 +1,5 @@ { "presubmit": { - "filter": "CredentialsTest.*:SurfaceFlingerStress.*:SurfaceInterceptorTest.*:LayerTransactionTest.*:LayerTypeTransactionTest.*:LayerUpdateTest.*:GeometryLatchingTest.*:CropLatchingTest.*:ChildLayerTest.*:ScreenCaptureTest.*:ScreenCaptureChildOnlyTest.*:DereferenceSurfaceControlTest.*:BoundlessLayerTest.*:MultiDisplayLayerBoundsTest.*:InvalidHandleTest.*" + "filter": "CredentialsTest.*:SurfaceFlingerStress.*:SurfaceInterceptorTest.*:LayerTransactionTest.*:LayerTypeTransactionTest.*:LayerUpdateTest.*:GeometryLatchingTest.*:CropLatchingTest.*:ChildLayerTest.*:ScreenCaptureTest.*:ScreenCaptureChildOnlyTest.*:DereferenceSurfaceControlTest.*:BoundlessLayerTest.*:MultiDisplayLayerBoundsTest.*" } } |