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

Commit 47c26e65 authored by Tom Cherry's avatar Tom Cherry Committed by Automerger Merge Worker
Browse files

Merge "Ensure that clearing the BlobCache sets mTotalSize to 0" am: 2f5adcb9...

Merge "Ensure that clearing the BlobCache sets mTotalSize to 0" am: 2f5adcb9 am: a47ecc53 am: 2879d60c am: f23a6a43

Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2451163



Change-Id: I21b13b4ad6c179f332c60c6b7575c25cb6c8ad79
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents 670e039d f23a6a43
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