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

Commit 2f5adcb9 authored by Tom Cherry's avatar Tom Cherry Committed by Gerrit Code Review
Browse files

Merge "Ensure that clearing the BlobCache sets mTotalSize to 0"

parents 2e38ba51 4fcb1c33
Loading
Loading
Loading
Loading
+3 −3
Original line number 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) {
    // All errors should result in the BlobCache being in an empty state.
    mCacheEntries.clear();
    clear();

    // Read the cache header
    if (size < sizeof(Header)) {
@@ -258,7 +258,7 @@ int BlobCache::unflatten(void const* buffer, size_t size) {
    size_t numEntries = header->mNumEntries;
    for (size_t i = 0; i < numEntries; i++) {
        if (byteOffset + sizeof(EntryHeader) > size) {
            mCacheEntries.clear();
            clear();
            ALOGE("unflatten: not enough room for cache entry headers");
            return -EINVAL;
        }
@@ -270,7 +270,7 @@ int BlobCache::unflatten(void const* buffer, size_t size) {

        size_t totalSize = align4(entrySize);
        if (byteOffset + totalSize > size) {
            mCacheEntries.clear();
            clear();
            ALOGE("unflatten: not enough room for cache entry headers");
            return -EINVAL;
        }
+4 −1
Original line number Diff line number Diff line
@@ -117,7 +117,10 @@ public:

    // clear flushes out all contents of the cache then the BlobCache, leaving
    // it in an empty state.
    void clear() { mCacheEntries.clear(); }
    void clear() {
        mCacheEntries.clear();
        mTotalSize = 0;
    }

protected:
    // mMaxTotalSize is the maximum size that all cache entries can occupy. This
+27 −0
Original line number Diff line number Diff line
@@ -466,4 +466,31 @@ TEST_F(BlobCacheFlattenTest, UnflattenCatchesBufferTooSmall) {
    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