Bug 3441: Part 1: Minimize cache size corruption by malformed swap.state.

If swap.state gets corrupted, a single entry with bogus size value will screw
up Squid idea of the current cache size. A newly added StoreSwapLogData sane()
method attempts to minimize the chance of corruption by ignoring log entries
with obviously bogus values.

However, without expensive size checks (-S or "Does the log entry matches the
actual cache file size?"), it is not possible to reliably detect all bogus log
entries.

If Squid gets a wrong idea of the current cache size, it may either cache too
much (and possibly run out of space) OR delete everything.

diff --git a/src/StoreSwapLogData.cc b/src/StoreSwapLogData.cc
index 1892cb53d3dfe637a8d54e0fb26917c4ca17ae76..11133b90095118b80b8c242f2d693fb5d1b0e0ea 100644 (file)
--- a/src/StoreSwapLogData.cc
+++ b/src/StoreSwapLogData.cc
@@ -39,6 +39,24 @@ StoreSwapLogData::StoreSwapLogData(): op(0), swap_filen (0), timestamp (0), last
     memset (key, '\0', sizeof(key));
 }
 
+bool
+StoreSwapLogData::sane() const
+{
+    // TODO: These checks are rather weak. A corrupted swap.state may still
+    // cause havoc (e.g., cur_size may become astronomical). Add checksums?
+
+    const time_t minTime = -2; // -1 is common; expires sometimes uses -2
+
+    // Check what we safely can; for some fields any value might be valid
+    return SWAP_LOG_NOP < op && op < SWAP_LOG_MAX &&
+           swap_filen >= 0 &&
+           timestamp >= minTime &&
+           lastref >= minTime &&
+           expires >= minTime &&
+           lastmod >= minTime &&
+           swap_file_sz > 0; // because swap headers ought to consume space
+}
+
 StoreSwapLogHeader::StoreSwapLogHeader():op(SWAP_LOG_VERSION), version(1)
 {
     record_size = sizeof(StoreSwapLogData);

diff --git a/src/StoreSwapLogData.h b/src/StoreSwapLogData.h
index 5fad46944fde8b61231e9a0d9696e365d7d8da9b..0ea668ab90a56ec6f52affbd92f930e11d8a78e4 100644 (file)
--- a/src/StoreSwapLogData.h
+++ b/src/StoreSwapLogData.h
@@ -86,6 +86,9 @@ public:
     MEMPROXY_CLASS(StoreSwapLogData);
     StoreSwapLogData();
 
+    /// consistency self-check: whether the data appears to make sense
+    bool sane() const;
+
     /**
      * Either SWAP_LOG_ADD when an object is added to the disk storage,
      * or SWAP_LOG_DEL when an object is deleted.

diff --git a/src/cache_diff.cc b/src/cache_diff.cc
index 782dc6f2a7ccd7fd991a3fdf4ea3df5cd79ef94d..4e919bd10b07036172942eb8b1b16dc24b250aec 100644 (file)
--- a/src/cache_diff.cc
+++ b/src/cache_diff.cc
@@ -180,7 +180,7 @@ cacheIndexScan(CacheIndex * idx, const char *fname, FILE * file)
     while (fread(&s, sizeof(s), 1, file) == 1) {
         count++;
         idx->scanned_count++;
-        /* if (s.op <= SWAP_LOG_NOP || s.op >= SWAP_LOG_MAX)
+        /* if (!s.sane())
          * continue; */
 
         if (s.op == SWAP_LOG_ADD) {

diff --git a/src/fs/ufs/ufscommon.cc b/src/fs/ufs/ufscommon.cc
index 6da30515330ce2e88e6397080e0c34673483faf5..c016e9c93a8d69c2775da585d09d848709695e97 100644 (file)
--- a/src/fs/ufs/ufscommon.cc
+++ b/src/fs/ufs/ufscommon.cc
@@ -604,11 +604,10 @@ RebuildState::rebuildFromSwapLog()
 
         n_read++;
 
-        if (swapData.op <= SWAP_LOG_NOP)
-            continue;
-
-        if (swapData.op >= SWAP_LOG_MAX)
+        if (!swapData.sane()) {
+            counts.invalid++;
             continue;
+        }
 
         /*
          * BC: during 2.4 development, we changed the way swap file