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

Commit e60f0b90 authored by Alec Mouri's avatar Alec Mouri
Browse files

Guard against overflow errors for transparent regions 

Applications may send arbitrary transparent regions to SurfaceFlinger
by overriding ViewGroup#gatherTransparentRegion, or a savvy binary may
apply a custom transparent region directly on a transaction. Arbitrary
transparent regions may cause SurfaceFlinger to crash on malformed
input.

We can guard against some of these crashes by constraining the
transparent region to be within the layer bounds

Bug: 230227800
Test: Stress tests
Test: libcompositionengine_test
Change-Id: I38f6f42dce38cdf2f34ba41af658adfdad290417
parent b4be2fb6

services/surfaceflinger/CompositionEngine/src/Output.cpp

+23 −2
Original line number Diff line number Diff line
@@ -586,8 +586,29 @@ void Output::ensureOutputLayerIfVisible(sp<compositionengine::LayerFE>& layerFE, 
    // Remove the transparent area from the visible region

    if (!layerFEState->isOpaque) {

        if (tr.preserveRects()) {

            // transform the transparent region

            transparentRegion = tr.transform(layerFEState->transparentRegionHint);

            // Clip the transparent region to geomLayerBounds first

            // The transparent region may be influenced by applications, for

            // instance, by overriding ViewGroup#gatherTransparentRegion with a

            // custom view. Once the layer stack -> display mapping is known, we

            // must guard against very wrong inputs to prevent underflow or

            // overflow errors. We do this here by constraining the transparent

            // region to be within the pre-transform layer bounds, since the

            // layer bounds are expected to play nicely with the full

            // transform.

            const Region clippedTransparentRegionHint =

                    layerFEState->transparentRegionHint.intersect(

                            Rect(layerFEState->geomLayerBounds));



            if (clippedTransparentRegionHint.isEmpty()) {

                if (!layerFEState->transparentRegionHint.isEmpty()) {

                    ALOGD("Layer: %s had an out of bounds transparent region",

                          layerFE->getDebugName());

                    layerFEState->transparentRegionHint.dump("transparentRegionHint");

                }

                transparentRegion.clear();

            } else {

                transparentRegion = tr.transform(clippedTransparentRegionHint);

            }

        } else {

            // transformation too complex, can't do the

            // transparent region optimization.

services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp

+41 −0
Original line number Diff line number Diff line
@@ -1505,6 +1505,8 @@ struct OutputEnsureOutputLayerIfVisibleTest : public testing::Test { 
    static const Region kTransparentRegionHint;

    static const Region kTransparentRegionHintTwo;

    static const Region kTransparentRegionHintTwo90Rotation;

    static const Region kTransparentRegionHintNegative;

    static const Region kTransparentRegionHintNegativeIntersectsBounds;



    StrictMock<OutputPartialMock> mOutput;

    LayerFESet mGeomSnapshots;
@@ -1528,6 +1530,10 @@ const Region OutputEnsureOutputLayerIfVisibleTest::kTransparentRegionHintTwo = 
        Region(Rect(25, 20, 50, 75));

const Region OutputEnsureOutputLayerIfVisibleTest::kTransparentRegionHintTwo90Rotation =

        Region(Rect(125, 25, 180, 50));

const Region OutputEnsureOutputLayerIfVisibleTest::kTransparentRegionHintNegative =

        Region(Rect(INT32_MIN, INT32_MIN, INT32_MIN + 100, INT32_MIN + 200));

const Region OutputEnsureOutputLayerIfVisibleTest::kTransparentRegionHintNegativeIntersectsBounds =

        Region(Rect(INT32_MIN, INT32_MIN, 100, 100));



TEST_F(OutputEnsureOutputLayerIfVisibleTest, performsGeomLatchBeforeCheckingIfLayerIncluded) {

    EXPECT_CALL(mOutput, includesLayer(sp<LayerFE>(mLayer.layerFE))).WillOnce(Return(false));
@@ -1997,6 +2003,41 @@ TEST_F(OutputEnsureOutputLayerIfVisibleTest, blockingRegionIsInOutputSpace) { 
                RegionEq(kTransparentRegionHintTwo90Rotation));

}



TEST_F(OutputEnsureOutputLayerIfVisibleTest, transparentRegionExcludesOutputLayer) {

    mLayer.layerFEState.isOpaque = false;

    mLayer.layerFEState.contentDirty = true;

    mLayer.layerFEState.geomLayerBounds = kFullBoundsNoRotation.bounds().toFloatRect();

    mLayer.layerFEState.transparentRegionHint = kFullBoundsNoRotation;



    EXPECT_CALL(mOutput, ensureOutputLayer(_, _)).Times(0);

}



TEST_F(OutputEnsureOutputLayerIfVisibleTest, transparentRegionIgnoredWhenOutsideBounds) {

    mLayer.layerFEState.isOpaque = false;

    mLayer.layerFEState.contentDirty = true;

    mLayer.layerFEState.geomLayerBounds = kFullBoundsNoRotation.bounds().toFloatRect();

    mLayer.layerFEState.transparentRegionHint = kTransparentRegionHintNegative;



    EXPECT_CALL(mOutput, ensureOutputLayer(_, _)).Times(0);

}



TEST_F(OutputEnsureOutputLayerIfVisibleTest, transparentRegionClipsWhenOutsideBounds) {

    mLayer.layerFEState.isOpaque = false;

    mLayer.layerFEState.contentDirty = true;

    mLayer.layerFEState.compositionType =

            aidl::android::hardware::graphics::composer3::Composition::DISPLAY_DECORATION;

    mLayer.layerFEState.transparentRegionHint = kTransparentRegionHintNegativeIntersectsBounds;



    EXPECT_CALL(mOutput, getOutputLayerCount()).WillOnce(Return(0u));

    EXPECT_CALL(mOutput, ensureOutputLayer(Eq(std::nullopt), Eq(mLayer.layerFE)))

            .WillOnce(Return(&mLayer.outputLayer));

    ensureOutputLayerIfVisible();



    // Check that the blocking region clips an out-of-bounds transparent region.

    EXPECT_THAT(mLayer.outputLayerState.outputSpaceBlockingRegionHint,

                RegionEq(kTransparentRegionHint));

}



/*

 * Output::present()

 */