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

Commit 0b81d3b2 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: I8265c60a16d5bd3dcf7f32e1533289b7cc5f2ad9
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents 9f6448ff 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