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

Commit 4fcb1c33 authored by Tom Cherry's avatar Tom Cherry
Browse files

Ensure that clearing the BlobCache sets mTotalSize to 0 

There is a subtle bug in BlobCache::unflatten() that can result in no
entries in mCacheEntries and mTotalSize >0 when the input file
contains valid entries followed by an invalid entry. This change
ensures that mTotalSize is set to 0 in that case.

Bug: 239862516
Bug: 269687033
Test: new unit test
Change-Id: Ieab1d7a98b96e4bc8ba8bc8a3c23dfe01c5eb896
parent e813ec15

opengl/libs/EGL/BlobCache.cpp

+3 −3
Original line number Original line Diff line number Diff line
@@ -231,7 +231,7 @@ int BlobCache::flatten(void* buffer, size_t size) const { 




int BlobCache::unflatten(void const* buffer, size_t size) {

int BlobCache::unflatten(void const* buffer, size_t size) {

    // All errors should result in the BlobCache being in an empty state.

    // All errors should result in the BlobCache being in an empty state.

    mCacheEntries.clear();

    clear();





    // Read the cache header

    // Read the cache header

    if (size < sizeof(Header)) {

    if (size < sizeof(Header)) {
@@ -258,7 +258,7 @@ int BlobCache::unflatten(void const* buffer, size_t size) { 
    size_t numEntries = header->mNumEntries;

    size_t numEntries = header->mNumEntries;

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

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

        if (byteOffset + sizeof(EntryHeader) > size) {

        if (byteOffset + sizeof(EntryHeader) > size) {

            mCacheEntries.clear();

            clear();

            ALOGE("unflatten: not enough room for cache entry headers");

            ALOGE("unflatten: not enough room for cache entry headers");

            return -EINVAL;

            return -EINVAL;

        }

        }
@@ -270,7 +270,7 @@ int BlobCache::unflatten(void const* buffer, size_t size) { 




        size_t totalSize = align4(entrySize);

        size_t totalSize = align4(entrySize);

        if (byteOffset + totalSize > size) {

        if (byteOffset + totalSize > size) {

            mCacheEntries.clear();

            clear();

            ALOGE("unflatten: not enough room for cache entry headers");

            ALOGE("unflatten: not enough room for cache entry headers");

            return -EINVAL;

            return -EINVAL;

        }

        }

opengl/libs/EGL/BlobCache.h

+4 −1
Original line number Original line Diff line number Diff line
@@ -117,7 +117,10 @@ public: 




    // clear flushes out all contents of the cache then the BlobCache, leaving

    // clear flushes out all contents of the cache then the BlobCache, leaving

    // it in an empty state.

    // it in an empty state.

    void clear() { mCacheEntries.clear(); }

    void clear() {

        mCacheEntries.clear();

        mTotalSize = 0;

    }





protected:

protected:

    // mMaxTotalSize is the maximum size that all cache entries can occupy. This

    // mMaxTotalSize is the maximum size that all cache entries can occupy. This

opengl/libs/EGL/BlobCache_test.cpp

+27 −0
Original line number Original line Diff line number Diff line
@@ -466,4 +466,31 @@ TEST_F(BlobCacheFlattenTest, UnflattenCatchesBufferTooSmall) { 
    ASSERT_EQ(size_t(0), mBC2->get("abcd", 4, buf, 4));

    ASSERT_EQ(size_t(0), mBC2->get("abcd", 4, buf, 4));

}

}





// Test for a divide by zero bug (b/239862516). Before the fix, unflatten() would not reset

// mTotalSize when it encountered an error, which would trigger division by 0 in clean() in the

// right conditions.

TEST_F(BlobCacheFlattenTest, SetAfterFailedUnflatten) {

    // isCleanable() must be true, so mTotalSize must be > mMaxTotalSize / 2 after unflattening

    // after one entry is lost. To make this the case, MaxTotalSize is 30 and three 10 sized

    // entries are used. One of those entries is lost, resulting in mTotalSize=20

    const size_t kMaxKeySize = 10;

    const size_t kMaxValueSize = 10;

    const size_t kMaxTotalSize = 30;

    mBC.reset(new BlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize));

    mBC2.reset(new BlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize));

    mBC->set("aaaaa", 5, "aaaaa", 5);

    mBC->set("bbbbb", 5, "bbbbb", 5);

    mBC->set("ccccc", 5, "ccccc", 5);



    size_t size = mBC->getFlattenedSize();

    uint8_t* flat = new uint8_t[size];

    ASSERT_EQ(OK, mBC->flatten(flat, size));



    ASSERT_EQ(BAD_VALUE, mBC2->unflatten(flat, size - 10));

    delete[] flat;



    // This line will trigger clean() which caused a crash.

    mBC2->set("dddddddddd", 10, "dddddddddd", 10);

}



} // namespace android
 
} // namespace android