Additional corruption instrumentation in safe-browsing filters.

chrome/browser/safe_browsing/safe_browsing_database.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/safe_browsing/safe_browsing_database.h"
 
 #include <algorithm>
 #include <iterator>
 
 #include "base/bind.h"
@@ skipping 278 matching lines @@
   PREFIX_SET_CREATE_ADD_PREFIXES_CHECKSUM,
   PREFIX_SET_CREATE_PREFIXES_CHECKSUM,
   PREFIX_SET_CREATE_GET_PREFIXES_CHECKSUM,
   // Failed checksums when probing the filters.
   PREFIX_SET_MISMATCH_PREFIX_SET_CHECKSUM,
   PREFIX_SET_MISMATCH_BLOOM_FILTER_CHECKSUM,
 
   // Recorded once per update if the impossible occurs.
   PREFIX_SET_BLOOM_MISS_PREFIX_HIT,
 
+  // Corrosponding CHECKSUM failed, but on retry it succeeded.
+  PREFIX_SET_CREATE_PREFIX_SET_CHECKSUM_SUCCEEDED,

[ramant (doing other things), 2012/08/15 00:07:57]

nit: Corrosponding -> Corresponding

+  PREFIX_SET_CREATE_BLOOM_FILTER_CHECKSUM_SUCCEEDED,
+
   // Memory space for histograms is determined by the max.  ALWAYS ADD
   // NEW VALUES BEFORE THIS ONE.
   PREFIX_SET_EVENT_MAX
 };
 
 void RecordPrefixSetInfo(PrefixSetEvent event_type) {
   UMA_HISTOGRAM_ENUMERATION("SB2.PrefixSetEvent", event_type,
                             PREFIX_SET_EVENT_MAX);
 }
 
@@ skipping 115 matching lines @@
 
     std::vector<SBPrefix> restored;
     prefix_set->get()->GetPrefixes(&restored);
     DCHECK_EQ(0, ChecksumPrefixes(prefix_set->get()->GetPrefixesChecksum(),
                                   restored));
   }
 
   // TODO(shess): Need a barrier to prevent ordering checks above here
   // or construction below here?
 
+  // NOTE(shess): So far, the fallout is:
+  // 605 PREFIX_SET_CHECKSUM (failed prefix_set->CheckChecksum()).
+  //  32 BLOOM_FILTER_CHECKSUM (failed bloom_filter->CheckChecksum()).
+  //   1 ADD_PREFIXES_CHECKSUM (contents of add_prefixes changed).
+  //   7 PREFIXES_CHECKSUM (contents of prefixes changed).
+
   bool get_prefixes_broken =
       (unique_prefixes_checksum != prefix_set->get()->GetPrefixesChecksum());
   bool prefix_set_broken = !prefix_set->get()->CheckChecksum();
   bool bloom_filter_broken = !bloom_filter->get()->CheckChecksum();
   bool prefixes_input_broken =
       (0 != ChecksumPrefixes(add_prefixes_checksum, prefixes));
   bool add_prefixes_input_broken =
       (0 != ChecksumAddPrefixes(add_prefixes_checksum, add_prefixes));
 
   if (add_prefixes_input_broken) {
     RecordPrefixSetInfo(PREFIX_SET_CREATE_ADD_PREFIXES_CHECKSUM);
   } else if (prefixes_input_broken) {
     RecordPrefixSetInfo(PREFIX_SET_CREATE_PREFIXES_CHECKSUM);
   }
 
-  if (prefix_set_broken)
+  if (prefix_set_broken) {
     RecordPrefixSetInfo(PREFIX_SET_CREATE_PREFIX_SET_CHECKSUM);
 
+    // Failing PrefixSet::CheckChecksum() implies that the set's
+    // internal data changed during construction.  If the retry
+    // consistently succeeds, that implies memory corruption.  If the
+    // retry consistently fails, that implies PrefixSet is broken.
+    scoped_ptr<safe_browsing::PrefixSet> retry_set(
+        new safe_browsing::PrefixSet(prefixes));
+    if (retry_set->CheckChecksum())
+      RecordPrefixSetInfo(PREFIX_SET_CREATE_PREFIX_SET_CHECKSUM_SUCCEEDED);
+
+    // TODO(shess): In case of double failure, consider pinning the
+    // data and calling NOTREACHED().  But it's a lot of data.  Could
+    // also implement a binary search to constrain the amount of input
+    // to consider, but that's a lot of complexity.
+  }
+
   // TODO(shess): Obviously this is a problem for operation of the
   // bloom filter!  But for purposes of checking prefix set operation,
   // all that matters is not messing up the histograms recorded later.
-  if (bloom_filter_broken)
+  if (bloom_filter_broken) {
     RecordPrefixSetInfo(PREFIX_SET_CREATE_BLOOM_FILTER_CHECKSUM);
 
+    // As for PrefixSet, failing BloomFilter::CheckChecksum() implies
+    // that the filter's internal data was changed.
+    scoped_refptr<BloomFilter> retry_filter(new BloomFilter(filter_size));
+    for (std::vector<SBPrefix>::const_iterator iter = prefixes.begin();
+         iter != prefixes.end(); ++iter) {
+      retry_filter->Insert(*iter);
+    }
+    if (retry_filter->CheckChecksum())
+      RecordPrefixSetInfo(PREFIX_SET_CREATE_BLOOM_FILTER_CHECKSUM_SUCCEEDED);
+  }
+
   // Attempt to isolate the case where the output of GetPrefixes()
   // would differ from the input presented.  This case implies that
   // PrefixSet has an actual implementation flaw, and may in the
   // future warrant more aggressive action, such as somehow dumping
   // |prefixes| up to the crash server.
   if (get_prefixes_broken && !prefix_set_broken && !prefixes_input_broken)
     RecordPrefixSetInfo(PREFIX_SET_CREATE_GET_PREFIXES_CHECKSUM);
 
   // If anything broke, clear the prefix set to prevent pollution of
   // histograms generated later.
@@ skipping 1088 matching lines @@
   if (std::binary_search(new_whitelist.begin(), new_whitelist.end(),
                          kill_switch)) {
     // The kill switch is whitelisted hence we whitelist all URLs.
     WhitelistEverything(whitelist);
   } else {
     base::AutoLock locked(lookup_lock_);
     whitelist->second = false;
     whitelist->first.swap(new_whitelist);
   }
 }