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

3 files changed, 34 insertions, 4 deletions

diff --git a/opengl/libs/EGL/BlobCache.cpp b/opengl/libs/EGL/BlobCache.cpp
index 86c788d3b3..aecfc6b077 100644
--- a/opengl/libs/EGL/BlobCache.cpp
+++ b/opengl/libs/EGL/BlobCache.cpp
@@ -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;
         }
diff --git a/opengl/libs/EGL/BlobCache.h b/opengl/libs/EGL/BlobCache.h
index ff03d30099..52078ff5fd 100644
--- a/opengl/libs/EGL/BlobCache.h
+++ b/opengl/libs/EGL/BlobCache.h
@@ -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
diff --git a/opengl/libs/EGL/BlobCache_test.cpp b/opengl/libs/EGL/BlobCache_test.cpp
index ceea0fb979..450c12837b 100644
--- a/opengl/libs/EGL/BlobCache_test.cpp
+++ b/opengl/libs/EGL/BlobCache_test.cpp
@@ -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