Add memory corruption checking to base::File().
Since we crash on invalid fd in ScopedFD::Close(), memory corruption
in a base::File() object can cause crashes; perhaps long after the memory
is actually corrupt.
This CL puts a checksum in memory near the fd on POSIX systems. The
checksum is different enough to be very unlikely to be correct after a
random memory write, but also very cheap to compute, so we can have
most operations on a file double quickly double check its integrity,
for earlier (and more useful for debugging) crashes.
This is intended to help us debug issue 424562. Once we have found out
if and where memory is being corrupted, this instrumentation code should
be removed.
R=thakis@chromium.org,pasko@chromium.org
BUG=424562
Review URL: https://codereview.chromium.org/702473009
Cr-Commit-Position: refs/heads/master@{#303251}
CrOS-Libchrome-Original-Commit: 32ea7f9a9a2385bd8911bf5bc37748a560a387d5
diff --git a/base/files/file_unittest.cc b/base/files/file_unittest.cc
index 6616f6a..9f57974 100644
--- a/base/files/file_unittest.cc
+++ b/base/files/file_unittest.cc
@@ -5,6 +5,7 @@
#include "base/files/file.h"
#include "base/files/file_util.h"
#include "base/files/scoped_temp_dir.h"
+#include "base/memory/scoped_ptr.h"
#include "base/time/time.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -466,3 +467,71 @@
EXPECT_EQ(0, info.size);
}
#endif // defined(OS_WIN)
+
+#if defined(OS_POSIX) && defined(GTEST_HAS_DEATH_TEST)
+TEST(FileTest, MemoryCorruption) {
+ {
+ // Test that changing the checksum value is detected.
+ base::File file;
+ EXPECT_NE(file.file_.file_memory_checksum_,
+ implicit_cast<unsigned int>(file.GetPlatformFile()));
+ file.file_.file_memory_checksum_ = file.GetPlatformFile();
+ EXPECT_DEATH(file.IsValid(), "corrupted fd memory");
+
+ file.file_.UpdateChecksum(); // Do not crash on File::~File().
+ }
+
+ {
+ // Test that changing the file descriptor value is detected.
+ base::File file;
+ file.file_.file_.reset(17);
+ EXPECT_DEATH(file.IsValid(), "corrupted fd memory");
+
+ // Do not crash on File::~File().
+ ignore_result(file.file_.file_.release());
+ file.file_.UpdateChecksum();
+ }
+
+ {
+ // Test that GetPlatformFile() checks for corruption.
+ base::File file;
+ file.file_.file_memory_checksum_ = file.GetPlatformFile();
+ EXPECT_DEATH(file.GetPlatformFile(), "corrupted fd memory");
+
+ file.file_.UpdateChecksum(); // Do not crash on File::~File().
+ }
+
+ {
+ // Test that the base::File destructor checks for corruption.
+ scoped_ptr<base::File> file(new File());
+ file->file_.file_memory_checksum_ = file->GetPlatformFile();
+ EXPECT_DEATH(file.reset(), "corrupted fd memory");
+
+ // Do not crash on this thread's destructor call.
+ file->file_.UpdateChecksum();
+ }
+
+ {
+ // Test that the base::File constructor checks for corruption.
+ base::File file;
+ file.file_.file_memory_checksum_ = file.GetPlatformFile();
+ EXPECT_DEATH(File f(file.Pass()), "corrupted fd memory");
+
+ file.file_.UpdateChecksum(); // Do not crash on File::~File().
+ }
+
+ {
+ // Test that doing IO checks for corruption.
+ base::File file;
+ file.file_.file_.reset(17); // A fake open FD value.
+
+ EXPECT_DEATH(file.Seek(File::FROM_BEGIN, 0), "corrupted fd memory");
+ EXPECT_DEATH(file.Read(0, NULL, 0), "corrupted fd memory");
+ EXPECT_DEATH(file.ReadAtCurrentPos(NULL, 0), "corrupted fd memory");
+ EXPECT_DEATH(file.Write(0, NULL, 0), "corrupted fd memory");
+
+ ignore_result(file.file_.file_.release());
+ file.file_.UpdateChecksum();
+ }
+}
+#endif // defined(OS_POSIX)