crash-reporter: add a sanity check for kernel dmesg records

On some devices, after a cold boot, a junk pstore record
/dev/pstore/dmesg-ramoops-0 is created which is just a chunk
of uninitialized memory containing random bits, and it's not
the result of a kernel crash.

The sanity check scans for the dmesg log level pattern to
avoid creating junk .kcrash files.

BUG=chromium:443764
TEST=platform_KernelErrorPaths with 3.8 and 3.14 kernel;
check no kcrash file is created for random binary ramoops dump
on stumpy.

Change-Id: I83041436cd8e5e0c7c0015c529f462032ce82f30
Signed-off-by: Ben Zhang <benzh@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/242147
Reviewed-by: Olof Johansson <olofj@chromium.org>
Reviewed-by: Grant Grundler <grundler@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

1 files changed, 16 insertions, 5 deletions

diff --git a/kernel_collector.cc b/kernel_collector.cc
index 98b3e8f..d86efbd 100644
--- a/kernel_collector.cc
+++ b/kernel_collector.cc
@@ -89,6 +89,8 @@ bool KernelCollector::ReadRecordToString(std::string *contents,
       "====\\d+\\.\\d+\n(.*)",
       pcrecpp::RE_Options().set_multiline(true).set_dotall(true));
 
+  pcrecpp::RE sanity_check_re("\n<\\d+>\\[\\s*(\\d+\\.\\d+)\\]");
+
   FilePath ramoops_record;
   GetRamoopsRecordPath(&ramoops_record, current_record);
   if (!base::ReadFileToString(ramoops_record, &record)) {
@@ -96,18 +98,27 @@ bool KernelCollector::ReadRecordToString(std::string *contents,
     return false;
   }
 
-  *record_found = true;
+  *record_found = false;
   if (record_re.FullMatch(record, &captured)) {
-    // Found a match, append it to the content, and remove from pstore.
+    // Found a ramoops header, so strip the header and append the rest.
     contents->append(captured);
-  } else {
+    *record_found = true;
+  } else if (sanity_check_re.PartialMatch(record.substr(0, 1024))) {
     // pstore compression has been added since kernel 3.12. In order to
     // decompress dmesg correctly, ramoops driver has to strip the header
     // before handing over the record to the pstore driver, so we don't
-    // need to do it here anymore.
+    // need to do it here anymore. However, the sanity check is needed because
+    // sometimes a pstore record is just a chunk of uninitialized memory which
+    // is not the result of a kernel crash. See crbug.com/443764
     contents->append(record);
+    *record_found = true;
+  } else {
+    LOG(WARNING) << "Found invalid record at " << ramoops_record.value();
   }
-  base::DeleteFile(ramoops_record, false);
+
+  // Remove the record from pstore after it's found.
+  if (*record_found)
+    base::DeleteFile(ramoops_record, false);
 
   return true;
 }