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

Commit 54cf5b1a authored by Robert Carr's avatar Robert Carr
Browse files

Fix crash when reparenting layer to itself. 

Currently this creates unbounded recursion and a SurfaceFlinger
crash.

Bug: 123283486
Test: Transaction_test.cpp
Change-Id: Ieae19cb3a7724de2ddb95a895328dd5b4a8d8782
parent a2e7f7f6

services/surfaceflinger/Layer.cpp

+13 −6
Original line number Diff line number Diff line
@@ -1623,19 +1623,26 @@ bool Layer::reparent(const sp<IBinder>& newParentHandle) { 
        return false;

    }



    sp<Layer> parent = getParent();

    if (parent != nullptr) {

        parent->removeChild(this);

    }



    sp<Layer> newParent;

    if (newParentHandle != nullptr) {

        auto handle = static_cast<Handle*>(newParentHandle.get());

        sp<Layer> newParent = handle->owner.promote();

        newParent = handle->owner.promote();

        if (newParent == nullptr) {

            ALOGE("Unable to promote Layer handle");

            return false;

        }

        if (newParent == this) {

            ALOGE("Invalid attempt to reparent Layer (%s) to itself", getName().c_str());

            return false;

        }

    }



    sp<Layer> parent = getParent();

    if (parent != nullptr) {

        parent->removeChild(this);

    }



    if (newParentHandle != nullptr) {

        newParent->addChild(this);

        if (!newParent->isRemovedFromCurrentState()) {

            addToCurrentState();

services/surfaceflinger/tests/Transaction_test.cpp

+17 −0
Original line number Diff line number Diff line
@@ -2460,6 +2460,23 @@ TEST_F(LayerTransactionTest, SetCachedBufferDelayed_BufferState) { 
    ASSERT_EQ(NO_ERROR, mClient->uncacheBuffer(bufferId));

}



TEST_F(LayerTransactionTest, ReparentToSelf) {

    sp<SurfaceControl> layer;

    ASSERT_NO_FATAL_FAILURE(layer = createLayer("test", 32, 32));

    ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(layer, Color::RED, 32, 32));

    Transaction().reparent(layer, layer->getHandle()).apply();



    {

        // We expect the transaction to be silently dropped, but for SurfaceFlinger

        // to still be functioning.

        SCOPED_TRACE("after reparent to self");

        const Rect rect(0, 0, 32, 32);

        auto shot = screenshot();

        shot->expectColor(rect, Color::RED);

        shot->expectBorder(rect, Color::BLACK);

    }

}



class ColorTransformHelper {

public:

    static void DegammaColorSingle(half& s) {