EGL BlobCache: Add CRC check to multifile

Apply a CRC check during file write, ensuring it is
the last thing applied to the file.

During initialization, verify the contents of the file
and remove if CRC doesn't match.

Add a new test (ModifiedCacheEndMisses) that modifies
the end of the entry and ensures no cache hit.

Test: pubg_mobile_launch ANGLE trace
Test: /data/nativetest64/EGL_test/EGL_test
Test: /data/nativetest64/libEGL_test/libEGL_test
Bug: b/267629017
Change-Id: I54015548b509c9ef2440e80f35b5ec97820a2144

6 files changed, 135 insertions, 42 deletions

diff --git a/opengl/libs/Android.bp b/opengl/libs/Android.bp
index 49e1cbafb4..16de3908f8 100644
--- a/opengl/libs/Android.bp
+++ b/opengl/libs/Android.bp
@@ -205,6 +205,7 @@ cc_test {
     srcs: [
         "EGL/BlobCache.cpp",
         "EGL/BlobCache_test.cpp",
+        "EGL/FileBlobCache.cpp",
         "EGL/MultifileBlobCache.cpp",
         "EGL/MultifileBlobCache_test.cpp",
     ],
diff --git a/opengl/libs/EGL/FileBlobCache.cpp b/opengl/libs/EGL/FileBlobCache.cpp
index 3f7ae7e5f4..1026842f87 100644
--- a/opengl/libs/EGL/FileBlobCache.cpp
+++ b/opengl/libs/EGL/FileBlobCache.cpp
@@ -31,7 +31,7 @@ static const size_t cacheFileHeaderSize = 8;
 
 namespace android {
 
-static uint32_t crc32c(const uint8_t* buf, size_t len) {
+uint32_t crc32c(const uint8_t* buf, size_t len) {
     const uint32_t polyBits = 0x82F63B78;
     uint32_t r = 0;
     for (size_t i = 0; i < len; i++) {
diff --git a/opengl/libs/EGL/FileBlobCache.h b/opengl/libs/EGL/FileBlobCache.h
index 8220723b2c..f083b0d6ca 100644
--- a/opengl/libs/EGL/FileBlobCache.h
+++ b/opengl/libs/EGL/FileBlobCache.h
@@ -22,6 +22,8 @@
 
 namespace android {
 
+uint32_t crc32c(const uint8_t* buf, size_t len);
+
 class FileBlobCache : public BlobCache {
 public:
     // FileBlobCache attempts to load the saved cache contents from disk into
diff --git a/opengl/libs/EGL/MultifileBlobCache.cpp b/opengl/libs/EGL/MultifileBlobCache.cpp
index 99af299f8d..607e2148e0 100644
--- a/opengl/libs/EGL/MultifileBlobCache.cpp
+++ b/opengl/libs/EGL/MultifileBlobCache.cpp
@@ -39,21 +39,10 @@
 
 using namespace std::literals;
 
-namespace {
-
-// Open the file and determine the size of the value it contains
-size_t getValueSizeFromFile(int fd, const std::string& entryPath) {
-    // Read the beginning of the file to get header
-    android::MultifileHeader header;
-    size_t result = read(fd, static_cast<void*>(&header), sizeof(android::MultifileHeader));
-    if (result != sizeof(android::MultifileHeader)) {
-        ALOGE("Error reading MultifileHeader from cache entry (%s): %s", entryPath.c_str(),
-              std::strerror(errno));
-        return 0;
-    }
+constexpr uint32_t kMultifileMagic = 'MFB$';
+constexpr uint32_t kCrcPlaceholder = 0;
 
-    return header.valueSize;
-}
+namespace {
 
 // Helper function to close entries or free them
 void freeHotCacheEntry(android::MultifileHotCache& entry) {
@@ -129,6 +118,15 @@ MultifileBlobCache::MultifileBlobCache(size_t maxTotalSize, size_t maxHotCacheSi
                     return;
                 }
 
+                // If the cache entry is damaged or no good, remove it
+                if (st.st_size <= 0 || st.st_atime <= 0) {
+                    ALOGE("INIT: Entry %u has invalid stats! Removing.", entryHash);
+                    if (remove(fullPath.c_str()) != 0) {
+                        ALOGE("Error removing %s: %s", fullPath.c_str(), std::strerror(errno));
+                    }
+                    continue;
+                }
+
                 // Open the file so we can read its header
                 int fd = open(fullPath.c_str(), O_RDONLY);
                 if (fd == -1) {
@@ -137,40 +135,67 @@ MultifileBlobCache::MultifileBlobCache(size_t maxTotalSize, size_t maxHotCacheSi
                     return;
                 }
 
-                // Look up the details we track about each file
-                size_t valueSize = getValueSizeFromFile(fd, fullPath);
+                // Read the beginning of the file to get header
+                MultifileHeader header;
+                size_t result = read(fd, static_cast<void*>(&header), sizeof(MultifileHeader));
+                if (result != sizeof(MultifileHeader)) {
+                    ALOGE("Error reading MultifileHeader from cache entry (%s): %s",
+                          fullPath.c_str(), std::strerror(errno));
+                    return;
+                }
 
-                // If the cache entry is damaged or no good, remove it
-                // TODO: Perform any other checks
-                if (valueSize <= 0 || st.st_size <= 0 || st.st_atime <= 0) {
-                    ALOGV("INIT: Entry %u has a problem! Removing.", entryHash);
+                // Verify header magic
+                if (header.magic != kMultifileMagic) {
+                    ALOGE("INIT: Entry %u has bad magic (%u)! Removing.", entryHash, header.magic);
                     if (remove(fullPath.c_str()) != 0) {
                         ALOGE("Error removing %s: %s", fullPath.c_str(), std::strerror(errno));
                     }
                     continue;
                 }
 
-                ALOGV("INIT: Entry %u is good, tracking it now.", entryHash);
-
                 // Note: Converting from off_t (signed) to size_t (unsigned)
                 size_t fileSize = static_cast<size_t>(st.st_size);
-                time_t accessTime = st.st_atime;
+
+                // Memory map the file
+                uint8_t* mappedEntry = reinterpret_cast<uint8_t*>(
+                        mmap(nullptr, fileSize, PROT_READ, MAP_PRIVATE, fd, 0));
+                if (mappedEntry == MAP_FAILED) {
+                    ALOGE("Failed to mmap cacheEntry, error: %s", std::strerror(errno));
+                    return;
+                }
+
+                // Ensure we have a good CRC
+                if (header.crc !=
+                    crc32c(mappedEntry + sizeof(MultifileHeader),
+                           fileSize - sizeof(MultifileHeader))) {
+                    ALOGE("INIT: Entry %u failed CRC check! Removing.", entryHash);
+                    if (remove(fullPath.c_str()) != 0) {
+                        ALOGE("Error removing %s: %s", fullPath.c_str(), std::strerror(errno));
+                    }
+                    continue;
+                }
+
+                // If the cache entry is damaged or no good, remove it
+                if (header.keySize <= 0 || header.valueSize <= 0) {
+                    ALOGE("INIT: Entry %u has a bad header keySize (%lu) or valueSize (%lu), "
+                          "removing.",
+                          entryHash, header.keySize, header.valueSize);
+                    if (remove(fullPath.c_str()) != 0) {
+                        ALOGE("Error removing %s: %s", fullPath.c_str(), std::strerror(errno));
+                    }
+                    continue;
+                }
+
+                ALOGV("INIT: Entry %u is good, tracking it now.", entryHash);
 
                 // Track details for rapid lookup later
-                trackEntry(entryHash, valueSize, fileSize, accessTime);
+                trackEntry(entryHash, header.valueSize, fileSize, st.st_atime);
 
                 // Track the total size
                 increaseTotalCacheSize(fileSize);
 
                 // Preload the entry for fast retrieval
                 if ((mHotCacheSize + fileSize) < mHotCacheLimit) {
-                    // Memory map the file
-                    uint8_t* mappedEntry = reinterpret_cast<uint8_t*>(
-                            mmap(nullptr, fileSize, PROT_READ, MAP_PRIVATE, fd, 0));
-                    if (mappedEntry == MAP_FAILED) {
-                        ALOGE("Failed to mmap cacheEntry, error: %s", std::strerror(errno));
-                    }
-
                     ALOGV("INIT: Populating hot cache with fd = %i, cacheEntry = %p for "
                           "entryHash %u",
                           fd, mappedEntry, entryHash);
@@ -183,6 +208,8 @@ MultifileBlobCache::MultifileBlobCache(size_t maxTotalSize, size_t maxHotCacheSi
                         return;
                     }
                 } else {
+                    // If we're not keeping it in hot cache, unmap it now
+                    munmap(mappedEntry, fileSize);
                     close(fd);
                 }
             }
@@ -247,10 +274,12 @@ void MultifileBlobCache::set(const void* key, EGLsizeiANDROID keySize, const voi
 
     uint8_t* buffer = new uint8_t[fileSize];
 
-    // Write the key and value after the header
-    android::MultifileHeader header = {keySize, valueSize};
+    // Write placeholders for magic and CRC until deferred thread complets the write
+    android::MultifileHeader header = {kMultifileMagic, kCrcPlaceholder, keySize, valueSize};
     memcpy(static_cast<void*>(buffer), static_cast<const void*>(&header),
            sizeof(android::MultifileHeader));
+
+    // Write the key and value after the header
     memcpy(static_cast<void*>(buffer + sizeof(MultifileHeader)), static_cast<const void*>(key),
            keySize);
     memcpy(static_cast<void*>(buffer + sizeof(MultifileHeader) + keySize),
@@ -600,6 +629,11 @@ void MultifileBlobCache::processTask(DeferredTask& task) {
 
             ALOGV("DEFERRED: Opened fd %i from %s", fd, fullPath.c_str());
 
+            // Add CRC check to the header (always do this last!)
+            MultifileHeader* header = reinterpret_cast<MultifileHeader*>(buffer);
+            header->crc =
+                    crc32c(buffer + sizeof(MultifileHeader), bufferSize - sizeof(MultifileHeader));
+
             ssize_t result = write(fd, buffer, bufferSize);
             if (result != bufferSize) {
                 ALOGE("Error writing fileSize to cache entry (%s): %s", fullPath.c_str(),
@@ -686,4 +720,4 @@ void MultifileBlobCache::waitForWorkComplete() {
     mWorkerIdleCondition.wait(lock, [this] { return (mTasks.empty() && mWorkerThreadIdle); });
 }
 
-}; // namespace android
\ No newline at end of file
+}; // namespace android
diff --git a/opengl/libs/EGL/MultifileBlobCache.h b/opengl/libs/EGL/MultifileBlobCache.h
index c0cc9dc2a9..cb105fba0d 100644
--- a/opengl/libs/EGL/MultifileBlobCache.h
+++ b/opengl/libs/EGL/MultifileBlobCache.h
@@ -28,9 +28,13 @@
 #include <unordered_map>
 #include <unordered_set>
 
+#include "FileBlobCache.h"
+
 namespace android {
 
 struct MultifileHeader {
+    uint32_t magic;
+    uint32_t crc;
     EGLsizeiANDROID keySize;
     EGLsizeiANDROID valueSize;
 };
diff --git a/opengl/tests/EGLTest/egl_cache_test.cpp b/opengl/tests/EGLTest/egl_cache_test.cpp
index 2b3e3a46af..ff4022cfe5 100644
--- a/opengl/tests/EGLTest/egl_cache_test.cpp
+++ b/opengl/tests/EGLTest/egl_cache_test.cpp
@@ -15,7 +15,7 @@
  */
 
 #define LOG_TAG "EGL_test"
-//#define LOG_NDEBUG 0
+// #define LOG_NDEBUG 0
 
 #include <gtest/gtest.h>
 
@@ -27,6 +27,7 @@
 #include "MultifileBlobCache.h"
 #include "egl_display.h"
 
+#include <fstream>
 #include <memory>
 
 using namespace std::literals;
@@ -144,7 +145,7 @@ std::string EGLCacheTest::getCachefileName() {
     return cachefileName;
 }
 
-TEST_P(EGLCacheTest, ModifiedCacheMisses) {
+TEST_P(EGLCacheTest, ModifiedCacheBeginMisses) {
     // Skip if not in multifile mode
     if (mCacheMode == egl_cache_t::EGLCacheMode::Monolithic) {
         GTEST_SKIP() << "Skipping test designed for multifile";
@@ -168,11 +169,12 @@ TEST_P(EGLCacheTest, ModifiedCacheMisses) {
     ASSERT_TRUE(cachefileName.length() > 0);
 
     // Stomp on the beginning of the cache file, breaking the key match
-    const long stomp = 0xbadf00d;
-    FILE *file = fopen(cachefileName.c_str(), "w");
-    fprintf(file, "%ld", stomp);
-    fflush(file);
-    fclose(file);
+    const char* stomp = "BADF00D";
+    std::fstream fs(cachefileName);
+    fs.seekp(0, std::ios_base::beg);
+    fs.write(stomp, strlen(stomp));
+    fs.flush();
+    fs.close();
 
     // Ensure no cache hit
     mCache->initialize(egl_display_t::get(EGL_DEFAULT_DISPLAY));
@@ -185,6 +187,56 @@ TEST_P(EGLCacheTest, ModifiedCacheMisses) {
     ASSERT_EQ(0xee, buf2[3]);
 }
 
+TEST_P(EGLCacheTest, ModifiedCacheEndMisses) {
+    // Skip if not in multifile mode
+    if (mCacheMode == egl_cache_t::EGLCacheMode::Monolithic) {
+        GTEST_SKIP() << "Skipping test designed for multifile";
+    }
+
+    uint8_t buf[16] = { 0xee, 0xee, 0xee, 0xee,
+                        0xee, 0xee, 0xee, 0xee,
+                        0xee, 0xee, 0xee, 0xee,
+                        0xee, 0xee, 0xee, 0xee };
+
+    mCache->initialize(egl_display_t::get(EGL_DEFAULT_DISPLAY));
+
+    mCache->setBlob("abcdefghij", 10, "klmnopqrstuvwxyz", 16);
+    ASSERT_EQ(16, mCache->getBlob("abcdefghij", 10, buf, 16));
+    ASSERT_EQ('w', buf[12]);
+    ASSERT_EQ('x', buf[13]);
+    ASSERT_EQ('y', buf[14]);
+    ASSERT_EQ('z', buf[15]);
+
+    // Ensure the cache file is written to disk
+    mCache->terminate();
+
+    // Depending on the cache mode, the file will be in different locations
+    std::string cachefileName = getCachefileName();
+    ASSERT_TRUE(cachefileName.length() > 0);
+
+    // Stomp on the END of the cache file, modifying its contents
+    const char* stomp = "BADF00D";
+    std::fstream fs(cachefileName);
+    fs.seekp(-strlen(stomp), std::ios_base::end);
+    fs.write(stomp, strlen(stomp));
+    fs.flush();
+    fs.close();
+
+    // Ensure no cache hit
+    mCache->initialize(egl_display_t::get(EGL_DEFAULT_DISPLAY));
+    uint8_t buf2[16] = { 0xee, 0xee, 0xee, 0xee,
+                         0xee, 0xee, 0xee, 0xee,
+                         0xee, 0xee, 0xee, 0xee,
+                         0xee, 0xee, 0xee, 0xee };
+
+    // getBlob may return junk for required size, but should not return a cache hit
+    mCache->getBlob("abcdefghij", 10, buf2, 16);
+    ASSERT_EQ(0xee, buf2[0]);
+    ASSERT_EQ(0xee, buf2[1]);
+    ASSERT_EQ(0xee, buf2[2]);
+    ASSERT_EQ(0xee, buf2[3]);
+}
+
 TEST_P(EGLCacheTest, TerminatedCacheBelowCacheLimit) {
     uint8_t buf[4] = { 0xee, 0xee, 0xee, 0xee };
     mCache->initialize(egl_display_t::get(EGL_DEFAULT_DISPLAY));