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

Commit 58da77ea authored by Steven Thomas's avatar Steven Thomas
Browse files

Avoid crash when setting buffer on deleted layer 

When setLayerBuffer() was called on a layer previously destroyed by
destroyLayer() the code would crash. Instead, return an error.

Bug: 37159844

Test: Ran vr flinger in a way that would trigger the crash, and
confirmed that I now get error logs instead of crashing. Unfortunately
the error code is consumed by the Composer wrapper class and not
propagated back to the caller, but that's a separate issue (b/37901601).

Change-Id: I75a5b954d47a1deac44d03851f60f347085eca89
parent 9f5b6aaf

graphics/composer/2.1/default/ComposerClient.cpp

+41 −18
Original line number Diff line number Diff line
@@ -655,18 +655,24 @@ bool ComposerClient::CommandReader::parseSetClientTarget(uint16_t length) 
    auto fence = readFence();

    auto dataspace = readSigned();

    auto damage = readRegion((length - 4) / 4);

    bool closeFence = true;



    auto err = lookupBuffer(BufferCache::CLIENT_TARGETS,

            slot, useCache, clientTarget, &clientTarget);

    if (err == Error::NONE) {

        err = mHal.setClientTarget(mDisplay, clientTarget, fence,

                dataspace, damage);



        updateBuffer(BufferCache::CLIENT_TARGETS, slot, useCache,

                clientTarget);

        auto updateBufErr = updateBuffer(BufferCache::CLIENT_TARGETS, slot,

                useCache, clientTarget);

        if (err == Error::NONE) {

            closeFence = false;

            err = updateBufErr;

        }

    if (err != Error::NONE) {

    }

    if (closeFence) {

        close(fence);

    }

    if (err != Error::NONE) {

        mWriter.setError(getCommandLoc(), err);

    }
@@ -683,17 +689,23 @@ bool ComposerClient::CommandReader::parseSetOutputBuffer(uint16_t length) 
    auto slot = read();

    auto outputBuffer = readHandle(&useCache);

    auto fence = readFence();

    bool closeFence = true;



    auto err = lookupBuffer(BufferCache::OUTPUT_BUFFERS,

            slot, useCache, outputBuffer, &outputBuffer);

    if (err == Error::NONE) {

        err = mHal.setOutputBuffer(mDisplay, outputBuffer, fence);



        updateBuffer(BufferCache::OUTPUT_BUFFERS,

        auto updateBufErr = updateBuffer(BufferCache::OUTPUT_BUFFERS,

                slot, useCache, outputBuffer);

        if (err == Error::NONE) {

            closeFence = false;

            err = updateBufErr;

        }

    if (err != Error::NONE) {

    }

    if (closeFence) {

        close(fence);

    }

    if (err != Error::NONE) {

        mWriter.setError(getCommandLoc(), err);

    }
@@ -786,17 +798,23 @@ bool ComposerClient::CommandReader::parseSetLayerBuffer(uint16_t length) 
    auto slot = read();

    auto buffer = readHandle(&useCache);

    auto fence = readFence();

    bool closeFence = true;



    auto err = lookupBuffer(BufferCache::LAYER_BUFFERS,

            slot, useCache, buffer, &buffer);

    if (err == Error::NONE) {

        err = mHal.setLayerBuffer(mDisplay, mLayer, buffer, fence);



        updateBuffer(BufferCache::LAYER_BUFFERS,

            slot, useCache, buffer);

        auto updateBufErr = updateBuffer(BufferCache::LAYER_BUFFERS, slot,

                useCache, buffer);

        if (err == Error::NONE) {

            closeFence = false;

            err = updateBufErr;

        }

    if (err != Error::NONE) {

    }

    if (closeFence) {

        close(fence);

    }

    if (err != Error::NONE) {

        mWriter.setError(getCommandLoc(), err);

    }
@@ -915,8 +933,10 @@ bool ComposerClient::CommandReader::parseSetLayerSidebandStream(uint16_t length) 
    auto err = lookupLayerSidebandStream(stream, &stream);

    if (err == Error::NONE) {

        err = mHal.setLayerSidebandStream(mDisplay, mLayer, stream);



        updateLayerSidebandStream(stream);

        auto updateErr = updateLayerSidebandStream(stream);

        if (err == Error::NONE) {

            err = updateErr;

        }

    }

    if (err != Error::NONE) {

        mWriter.setError(getCommandLoc(), err);
@@ -1097,21 +1117,24 @@ Error ComposerClient::CommandReader::lookupBuffer(BufferCache cache, 
    return Error::NONE;

}



void ComposerClient::CommandReader::updateBuffer(BufferCache cache,

Error ComposerClient::CommandReader::updateBuffer(BufferCache cache,

        uint32_t slot, bool useCache, buffer_handle_t handle)

{

    // handle was looked up from cache

    if (useCache) {

        return;

        return Error::NONE;

    }



    std::lock_guard<std::mutex> lock(mClient.mDisplayDataMutex);



    BufferCacheEntry* entry = nullptr;

    lookupBufferCacheEntryLocked(cache, slot, &entry);

    LOG_FATAL_IF(!entry, "failed to find the cache entry to update");

    Error error = lookupBufferCacheEntryLocked(cache, slot, &entry);

    if (error != Error::NONE) {

      return error;

    }



    *entry = handle;

    return Error::NONE;

}



} // namespace implementation

graphics/composer/2.1/default/ComposerClient.h

+3 −3
Original line number Diff line number Diff line
@@ -173,7 +173,7 @@ protected: 
        Error lookupBuffer(BufferCache cache, uint32_t slot,

                bool useCache, buffer_handle_t handle,

                buffer_handle_t* outHandle);

        void updateBuffer(BufferCache cache, uint32_t slot,

        Error updateBuffer(BufferCache cache, uint32_t slot,

                bool useCache, buffer_handle_t handle);



        Error lookupLayerSidebandStream(buffer_handle_t handle,
@@ -182,9 +182,9 @@ protected: 
            return lookupBuffer(BufferCache::LAYER_SIDEBAND_STREAMS,

                    0, false, handle, outHandle);

        }

        void updateLayerSidebandStream(buffer_handle_t handle)

        Error updateLayerSidebandStream(buffer_handle_t handle)

        {

            updateBuffer(BufferCache::LAYER_SIDEBAND_STREAMS,

            return updateBuffer(BufferCache::LAYER_SIDEBAND_STREAMS,

                    0, false, handle);

        }