Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1c5533d4 authored by Ben Zhang's avatar Ben Zhang Committed by ChromeOS Commit Bot
Browse files

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: default avatarBen Zhang <benzh@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/242147


Reviewed-by: default avatarOlof Johansson <olofj@chromium.org>
Reviewed-by: default avatarGrant Grundler <grundler@chromium.org>
Reviewed-by: default avatarMike Frysinger <vapier@chromium.org>
parent 859ee45c

crash_reporter/kernel_collector.cc

+16 −5
Original line number Diff line number Diff line
@@ -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,17 +98,26 @@ 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();

  }



  // Remove the record from pstore after it's found.

  if (*record_found)

    base::DeleteFile(ramoops_record, false);



  return true;

crash_reporter/kernel_collector_test.cc

+8 −2
Original line number Diff line number Diff line
@@ -78,15 +78,21 @@ TEST_F(KernelCollectorTest, LoadPreservedDump) { 
  std::string dump;

  dump.clear();



  WriteStringToFile(kcrash_file(), "CrashRecordWithoutRamoopsHeader");

  WriteStringToFile(kcrash_file(),

      "CrashRecordWithoutRamoopsHeader\n<6>[    0.078852]");

  ASSERT_TRUE(collector_.LoadParameters());

  ASSERT_TRUE(collector_.LoadPreservedDump(&dump));

  ASSERT_EQ("CrashRecordWithoutRamoopsHeader", dump);

  ASSERT_EQ("CrashRecordWithoutRamoopsHeader\n<6>[    0.078852]", dump);



  WriteStringToFile(kcrash_file(), "====1.1\nsomething");

  ASSERT_TRUE(collector_.LoadParameters());

  ASSERT_TRUE(collector_.LoadPreservedDump(&dump));

  ASSERT_EQ("something", dump);



  WriteStringToFile(kcrash_file(), "\x01\x02\xfe\xff random blob");

  ASSERT_TRUE(collector_.LoadParameters());

  ASSERT_FALSE(collector_.LoadPreservedDump(&dump));

  ASSERT_EQ("", dump);

}



TEST_F(KernelCollectorTest, EnableMissingKernel) {