Fixed wrong assumption in Table::Open()

Summary:
`Table::Open()` assumes that `size` correctly describes the size of `file`, added a check that the footer is actually the right size and for good measure added assertions to `Footer::DecodeFrom()`.

This was discovered by running `valgrind ./db_test` and seeing that `Footer::DecodeFrom()` was accessing uninitialized memory.

Test Plan:
make clean check

ran `valgrind ./db_test` and saw DBTest.NoSpace no longer complains about a conditional jump being dependent on uninitialized memory.

Reviewers: dhruba, vamsi, emayanke, sheki

Reviewed By: dhruba

CC: leveldb
Differential Revision: https://reviews.facebook.net/D7815

diff --git a/table/format.cc b/table/format.cc
index baa48db4bb0e8b1962ebd2813d20ea9a7d9325be..d847d409299730fc541597642dfcbb2a21421ffe 100644 (file)
--- a/table/format.cc
+++ b/table/format.cc
@@ -42,6 +42,9 @@ void Footer::EncodeTo(std::string* dst) const {
 }
 
 Status Footer::DecodeFrom(Slice* input) {
+  assert(input != NULL);
+  assert(input->size() >= kEncodedLength);
+
   const char* magic_ptr = input->data() + kEncodedLength - 8;
   const uint32_t magic_lo = DecodeFixed32(magic_ptr);
   const uint32_t magic_hi = DecodeFixed32(magic_ptr + 4);

diff --git a/table/table.cc b/table/table.cc
index 062bf7abd9e8ccde728eee0fe78a316320de9018..394f65e7d9e6437d8ccab4c41e05200e21358e38 100644 (file)
--- a/table/table.cc
+++ b/table/table.cc
@@ -51,6 +51,13 @@ Status Table::Open(const Options& options,
                         &footer_input, footer_space);
   if (!s.ok()) return s;
 
+  // Check that we actually read the whole footer from the file. It may be
+  // that size isn't correct.
+  if (footer_input.size() != Footer::kEncodedLength) {
+    return Status::InvalidArgument("file is too short to be an sstable");
+  }
+
+
   Footer footer;
   s = footer.DecodeFrom(&footer_input);
   if (!s.ok()) return s;