Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3190813d authored by Patrick Williams's avatar Patrick Williams
Browse files

Fix RenderArea use after free error 

When RenderEngine is single threaded, renderScreenImpl immediately calls present instead of moving it into the returned future. This caused the unique_ptr<RenderArea> to be destroyed when renderScreenImpl returned, deleting RenderArea prematurely. We can use a shared_ptr to ensure that RenderArea is only deleted once both captureScreenCommon and present no longer need it.

Bug: 264827546
Test: atest ScreenCaptureTest
Test: go/wm-smoke
Change-Id: I8b591bec7c6ad07df9fa9b7f62495dac10a68a93
parent d26c1061

services/surfaceflinger/SurfaceFlinger.cpp

+3 −3
Original line number Original line Diff line number Diff line
@@ -6509,7 +6509,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenCommon( 
            [=, renderAreaFuture = std::move(renderAreaFuture)]() FTL_FAKE_GUARD(

            [=, renderAreaFuture = std::move(renderAreaFuture)]() FTL_FAKE_GUARD(

                    kMainThreadContext) mutable -> ftl::SharedFuture<FenceResult> {

                    kMainThreadContext) mutable -> ftl::SharedFuture<FenceResult> {

                ScreenCaptureResults captureResults;

                ScreenCaptureResults captureResults;

                std::unique_ptr<RenderArea> renderArea = renderAreaFuture.get();

                std::shared_ptr<RenderArea> renderArea = renderAreaFuture.get();

                if (!renderArea) {

                if (!renderArea) {

                    ALOGW("Skipping screen capture because of invalid render area.");

                    ALOGW("Skipping screen capture because of invalid render area.");

                    if (captureListener) {

                    if (captureListener) {
@@ -6521,7 +6521,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenCommon( 




                ftl::SharedFuture<FenceResult> renderFuture;

                ftl::SharedFuture<FenceResult> renderFuture;

                renderArea->render([&]() FTL_FAKE_GUARD(kMainThreadContext) {

                renderArea->render([&]() FTL_FAKE_GUARD(kMainThreadContext) {

                    renderFuture = renderScreenImpl(std::move(renderArea), traverseLayers, buffer,

                    renderFuture = renderScreenImpl(renderArea, traverseLayers, buffer,

                                                    canCaptureBlackoutContent, regionSampling,

                                                    canCaptureBlackoutContent, regionSampling,

                                                    grayscale, captureResults);

                                                    grayscale, captureResults);

                });

                });
@@ -6549,7 +6549,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenCommon( 
}

}





ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl(

ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl(

        std::unique_ptr<RenderArea> renderArea, TraverseLayersFunction traverseLayers,

        std::shared_ptr<const RenderArea> renderArea, TraverseLayersFunction traverseLayers,

        const std::shared_ptr<renderengine::ExternalTexture>& buffer,

        const std::shared_ptr<renderengine::ExternalTexture>& buffer,

        bool canCaptureBlackoutContent, bool regionSampling, bool grayscale,

        bool canCaptureBlackoutContent, bool regionSampling, bool grayscale,

        ScreenCaptureResults& captureResults) {

        ScreenCaptureResults& captureResults) {

services/surfaceflinger/SurfaceFlinger.h

+1 −1
Original line number Original line Diff line number Diff line
@@ -797,7 +797,7 @@ private: 
            const std::shared_ptr<renderengine::ExternalTexture>&, bool regionSampling,

            const std::shared_ptr<renderengine::ExternalTexture>&, bool regionSampling,

            bool grayscale, const sp<IScreenCaptureListener>&);

            bool grayscale, const sp<IScreenCaptureListener>&);

    ftl::SharedFuture<FenceResult> renderScreenImpl(

    ftl::SharedFuture<FenceResult> renderScreenImpl(

            std::unique_ptr<RenderArea>, TraverseLayersFunction,

            std::shared_ptr<const RenderArea>, TraverseLayersFunction,

            const std::shared_ptr<renderengine::ExternalTexture>&, bool canCaptureBlackoutContent,

            const std::shared_ptr<renderengine::ExternalTexture>&, bool canCaptureBlackoutContent,

            bool regionSampling, bool grayscale, ScreenCaptureResults&) EXCLUDES(mStateLock)

            bool regionSampling, bool grayscale, ScreenCaptureResults&) EXCLUDES(mStateLock)

            REQUIRES(kMainThreadContext);

            REQUIRES(kMainThreadContext);

services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h

+1 −1
Original line number Original line Diff line number Diff line
@@ -401,7 +401,7 @@ public: 
        return mFlinger->setPowerModeInternal(display, mode);

        return mFlinger->setPowerModeInternal(display, mode);

    }

    }





    auto renderScreenImpl(std::unique_ptr<RenderArea> renderArea,

    auto renderScreenImpl(std::shared_ptr<const RenderArea> renderArea,

                          SurfaceFlinger::TraverseLayersFunction traverseLayers,

                          SurfaceFlinger::TraverseLayersFunction traverseLayers,

                          const std::shared_ptr<renderengine::ExternalTexture>& buffer,

                          const std::shared_ptr<renderengine::ExternalTexture>& buffer,

                          bool forSystem, bool regionSampling) {

                          bool forSystem, bool regionSampling) {