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

Commit cacfcc66 authored by Chia-I Wu's avatar Chia-I Wu
Browse files

libgui: add helper to find CpuConsumer::AcquiredBuffer 

Add CpuConsumer::findAcquiredBufferLocked.  Rename mBufferPointer to
mLockedBufferId.

This also fixes a nullptr-dereference when unlockBuffer is called
with a LockedBuffer whose data is nullptr.

Test: libgui_test
Change-Id: I36c665099f145731197a6ff8ab940c5d7a326df8
parent e3362a79

libs/gui/CpuConsumer.cpp

+28 −22
Original line number Diff line number Diff line
@@ -60,6 +60,21 @@ void CpuConsumer::setName(const String8& name) { 
    mConsumer->setConsumerName(name);

}



size_t CpuConsumer::findAcquiredBufferLocked(uintptr_t id) const {

    for (size_t i = 0; i < mMaxLockedBuffers; i++) {

        const auto& ab = mAcquiredBuffers[i];

        // note that this finds AcquiredBuffer::kUnusedId as well

        if (ab.mLockedBufferId == id) {

            return i;

        }

    }

    return mMaxLockedBuffers; // an invalid index

}



static uintptr_t getLockedBufferId(const CpuConsumer::LockedBuffer& buffer) {

    return reinterpret_cast<uintptr_t>(buffer.data);

}



static bool isPossiblyYUV(PixelFormat format) {

    switch (static_cast<int>(format)) {

        case HAL_PIXEL_FORMAT_RGBA_8888:
@@ -165,20 +180,6 @@ status_t CpuConsumer::lockNextBuffer(LockedBuffer *nativeBuffer) { 
        }

    }



    size_t lockedIdx = 0;

    for (; lockedIdx < static_cast<size_t>(mMaxLockedBuffers); lockedIdx++) {

        if (mAcquiredBuffers[lockedIdx].mSlot ==

                BufferQueue::INVALID_BUFFER_SLOT) {

            break;

        }

    }

    assert(lockedIdx < mMaxLockedBuffers);



    AcquiredBuffer &ab = mAcquiredBuffers.editItemAt(lockedIdx);

    ab.mSlot = slot;

    ab.mBufferPointer = bufferPointer;

    ab.mGraphicBuffer = mSlots[slot].mGraphicBuffer;



    nativeBuffer->data   =

            reinterpret_cast<uint8_t*>(bufferPointer);

    nativeBuffer->width  = mSlots[slot].mGraphicBuffer->getWidth();
@@ -201,6 +202,15 @@ status_t CpuConsumer::lockNextBuffer(LockedBuffer *nativeBuffer) { 
    nativeBuffer->chromaStride = static_cast<uint32_t>(ycbcr.cstride);

    nativeBuffer->chromaStep   = static_cast<uint32_t>(ycbcr.chroma_step);



    // find an unused AcquiredBuffer

    size_t lockedIdx = findAcquiredBufferLocked(AcquiredBuffer::kUnusedId);

    ALOG_ASSERT(lockedIdx < mMaxLockedBuffers);

    AcquiredBuffer& ab = mAcquiredBuffers.editItemAt(lockedIdx);



    ab.mSlot = slot;

    ab.mGraphicBuffer = mSlots[slot].mGraphicBuffer;

    ab.mLockedBufferId = getLockedBufferId(*nativeBuffer);



    mCurrentLockedBuffers++;



    return OK;
@@ -208,12 +218,10 @@ status_t CpuConsumer::lockNextBuffer(LockedBuffer *nativeBuffer) { 


status_t CpuConsumer::unlockBuffer(const LockedBuffer &nativeBuffer) {

    Mutex::Autolock _l(mMutex);

    size_t lockedIdx = 0;



    void *bufPtr = reinterpret_cast<void *>(nativeBuffer.data);

    for (; lockedIdx < static_cast<size_t>(mMaxLockedBuffers); lockedIdx++) {

        if (bufPtr == mAcquiredBuffers[lockedIdx].mBufferPointer) break;

    }

    uintptr_t id = getLockedBufferId(nativeBuffer);

    size_t lockedIdx =

        (id != AcquiredBuffer::kUnusedId) ? findAcquiredBufferLocked(id) : mMaxLockedBuffers;

    if (lockedIdx == mMaxLockedBuffers) {

        CC_LOGE("%s: Can't find buffer to free", __FUNCTION__);

        return BAD_VALUE;
@@ -252,9 +260,7 @@ status_t CpuConsumer::releaseAcquiredBufferLocked(size_t lockedIdx) { 
    }



    AcquiredBuffer &ab = mAcquiredBuffers.editItemAt(lockedIdx);

    ab.mSlot = BufferQueue::INVALID_BUFFER_SLOT;

    ab.mBufferPointer = NULL;

    ab.mGraphicBuffer.clear();

    ab.reset();



    mCurrentLockedBuffers--;

    return OK;

libs/gui/include/gui/CpuConsumer.h

+13 −2
Original line number Diff line number Diff line
@@ -127,18 +127,29 @@ class CpuConsumer : public ConsumerBase 


    // Tracking for buffers acquired by the user

    struct AcquiredBuffer {

        static constexpr uintptr_t kUnusedId = 0;



        // Need to track the original mSlot index and the buffer itself because

        // the mSlot entry may be freed/reused before the acquired buffer is

        // released.

        int mSlot;

        sp<GraphicBuffer> mGraphicBuffer;

        void *mBufferPointer;

        uintptr_t mLockedBufferId;



        AcquiredBuffer() :

                mSlot(BufferQueue::INVALID_BUFFER_SLOT),

                mBufferPointer(NULL) {

                mLockedBufferId(kUnusedId) {

        }



        void reset() {

            mSlot = BufferQueue::INVALID_BUFFER_SLOT;

            mGraphicBuffer.clear();

            mLockedBufferId = kUnusedId;

        }

    };



    size_t findAcquiredBufferLocked(uintptr_t id) const;



    Vector<AcquiredBuffer> mAcquiredBuffers;



    // Count of currently locked buffers

libs/gui/tests/CpuConsumer_test.cpp

+9 −0
Original line number Diff line number Diff line
@@ -681,6 +681,15 @@ TEST_P(CpuConsumerTest, FromCpuLockMax) { 
    }

}



TEST_P(CpuConsumerTest, FromCpuInvalid) {

    status_t err = mCC->lockNextBuffer(nullptr);

    ASSERT_EQ(BAD_VALUE, err) << "lockNextBuffer did not fail";



    CpuConsumer::LockedBuffer b;

    err = mCC->unlockBuffer(b);

    ASSERT_EQ(BAD_VALUE, err) << "unlockBuffer did not fail";

}



CpuConsumerTestParams y8TestSets[] = {

    { 512,   512, 1, HAL_PIXEL_FORMAT_Y8},

    { 512,   512, 3, HAL_PIXEL_FORMAT_Y8},