Import Cobalt 2.15147 Change-Id: I9a11d41d571fb25d2a0f9b9fd386d661f7d4ce71
diff --git a/src/cobalt/build/build.id b/src/cobalt/build/build.id index b3eeb13..159bca5 100644 --- a/src/cobalt/build/build.id +++ b/src/cobalt/build/build.id
@@ -1 +1 @@ -13610 \ No newline at end of file +15147 \ No newline at end of file
diff --git a/src/cobalt/storage/storage_manager.cc b/src/cobalt/storage/storage_manager.cc index decba88..79a5c8f 100644 --- a/src/cobalt/storage/storage_manager.cc +++ b/src/cobalt/storage/storage_manager.cc
@@ -275,11 +275,9 @@ sizeof(VirtualFileSystem::SerializedHeader)); } - if (VirtualFileSystem::GetHeaderVersion(header) == -1) { + if (!vfs_->Deserialize(&raw_bytes[0], buffer_size)) { VirtualFile* vf = vfs_->Open(kDefaultSaveFile); vf->Write(&raw_bytes[0], buffer_size, 0 /* offset */); - } else { - vfs_->Deserialize(&raw_bytes[0], buffer_size); } } }
diff --git a/src/cobalt/storage/virtual_file.cc b/src/cobalt/storage/virtual_file.cc index ee738da..64904c2 100644 --- a/src/cobalt/storage/virtual_file.cc +++ b/src/cobalt/storage/virtual_file.cc
@@ -27,60 +27,74 @@ // We must use forward declarations to be able to specify the // WARN_UNUSED_RESULT attribute. -// Read size bytes from buffer into dest. Return # of bytes read. -int ReadBuffer(uint8* dest, const uint8* buffer, int size) WARN_UNUSED_RESULT; +// Read size bytes from buffer into dest. Updates buffer and buffer_remaining +// on success. +// Returns true on success, false on buffer overrun +bool ReadBuffer(uint8* dest, const uint8** buffer, size_t size, + size_t* buffer_remaining) WARN_UNUSED_RESULT; // Write size bytes from source into buffer. Return # of bytes written. -int WriteBuffer(uint8* buffer, const uint8* source, int size, - bool dry_run) WARN_UNUSED_RESULT; +size_t WriteBuffer(uint8* buffer, const uint8* source, size_t size, + bool dry_run) WARN_UNUSED_RESULT; -int ReadBuffer(uint8* dest, const uint8* buffer, int size) { - memcpy(dest, buffer, static_cast<size_t>(size)); - return size; +bool ReadBuffer(uint8* dest, const uint8** buffer, size_t size, + size_t* buffer_remaining) { + if (size > *buffer_remaining) { + return false; + } + memcpy(dest, *buffer, size); + *buffer_remaining -= size; + *buffer += size; + return true; } -int WriteBuffer(uint8* buffer, const uint8* source, int size, bool dry_run) { +size_t WriteBuffer(uint8* buffer, const uint8* source, size_t size, + bool dry_run) { if (!dry_run) { - memcpy(buffer, source, static_cast<size_t>(size)); + memcpy(buffer, source, size); } return size; } } // namespace -VirtualFile::VirtualFile(const std::string& name) : size_(0), name_(name) {} +VirtualFile::VirtualFile(const std::string& name) : name_(name) {} VirtualFile::~VirtualFile() {} -int VirtualFile::Read(void* dest, int bytes, int offset) const { - DCHECK_GE(offset, 0); - if (offset > size_) { +int VirtualFile::Read(void* dest, int bytes_in, int offset_in) const { + DCHECK_GE(offset_in, 0); + DCHECK_GE(bytes_in, 0); + size_t offset = static_cast<size_t>(offset_in); + size_t bytes = static_cast<size_t>(bytes_in); + size_t size = buffer_.size(); + if (offset > size) { return 0; } - int bytes_to_read = std::min(size_ - offset, bytes); + size_t bytes_to_read = std::min(static_cast<size_t>(size - offset), bytes); if (bytes_to_read == 0) { return 0; } - memcpy(dest, &buffer_[static_cast<size_t>(offset)], - static_cast<size_t>(bytes_to_read)); - return bytes_to_read; + memcpy(dest, &buffer_[offset], bytes_to_read); + return static_cast<int>(bytes_to_read); } -int VirtualFile::Write(const void* source, int bytes, int offset) { - DCHECK_GE(offset, 0); - DCHECK_GE(bytes, 0); - if (static_cast<int>(buffer_.size()) < offset + bytes) { - buffer_.resize(static_cast<size_t>(offset + bytes)); +int VirtualFile::Write(const void* source, int bytes_in, int offset_in) { + DCHECK_GE(offset_in, 0); + DCHECK_GE(bytes_in, 0); + size_t bytes = static_cast<size_t>(bytes_in); + size_t offset = static_cast<size_t>(offset_in); + if (buffer_.size() < offset + bytes) { + buffer_.resize(offset + bytes); } - memcpy(&buffer_[static_cast<size_t>(offset)], source, - static_cast<size_t>(bytes)); - size_ = std::max(size_, offset + bytes); - return bytes; + memcpy(&buffer_[offset], source, bytes); + return bytes_in; } int VirtualFile::Truncate(int size) { - size_ = std::min(size_, size); - return size_; + size_t newsize = std::min(buffer_.size(), static_cast<size_t>(size)); + buffer_.resize(newsize); + return static_cast<int>(newsize); } int VirtualFile::Serialize(uint8* dest, bool dry_run) { @@ -96,29 +110,36 @@ // Write the filename cur_buffer += WriteBuffer(cur_buffer, reinterpret_cast<const uint8*>(name_.c_str()), - static_cast<int>(name_length), dry_run); + static_cast<size_t>(name_length), dry_run); // NOTE: Ensure the file size is 64-bit for compatibility // with any existing serialized files. - uint64 file_size = static_cast<uint64>(size_); + uint64 file_size = static_cast<uint64>(buffer_.size()); // Write the file contents size cur_buffer += WriteBuffer(cur_buffer, reinterpret_cast<const uint8*>(&file_size), sizeof(file_size), dry_run); // Write the file contents - cur_buffer += WriteBuffer(cur_buffer, &buffer_[0], size_, dry_run); + if (!buffer_.empty()) { + // std::vector does not define access to underlying array when empty + cur_buffer += WriteBuffer(cur_buffer, &buffer_[0], buffer_.size(), dry_run); + } // Return the number of bytes written return static_cast<int>(std::distance(dest, cur_buffer)); } -int VirtualFile::Deserialize(const uint8* source) { +int VirtualFile::Deserialize(const uint8* source, size_t buffer_remaining) { // Read in filename length const uint8* cur_buffer = source; uint64 name_length; - cur_buffer += ReadBuffer(reinterpret_cast<uint8*>(&name_length), cur_buffer, - sizeof(name_length)); + bool success = ReadBuffer(reinterpret_cast<uint8*>(&name_length), &cur_buffer, + sizeof(name_length), &buffer_remaining); + if (!success) { + DLOG(ERROR) << "Buffer overrun"; + return -1; + } if (name_length >= kMaxVfsPathname) { DLOG(ERROR) << "Filename was longer than the maximum allowed."; @@ -126,19 +147,34 @@ } // Read in filename char name[kMaxVfsPathname]; - cur_buffer += ReadBuffer(reinterpret_cast<uint8*>(name), cur_buffer, - static_cast<int>(name_length)); + success = ReadBuffer(reinterpret_cast<uint8*>(name), &cur_buffer, + static_cast<size_t>(name_length), &buffer_remaining); + if (!success) { + DLOG(ERROR) << "Buffer overrun"; + return -1; + } name_.assign(name, name_length); // Read in file contents size. uint64 file_size; - cur_buffer += ReadBuffer(reinterpret_cast<uint8*>(&file_size), cur_buffer, - sizeof(file_size)); - size_ = static_cast<int>(file_size); - + success = ReadBuffer(reinterpret_cast<uint8*>(&file_size), &cur_buffer, + sizeof(file_size), &buffer_remaining); + if (!success) { + DLOG(ERROR) << "Buffer overrun"; + return -1; + } // Read in the file contents - buffer_.resize(static_cast<size_t>(size_)); - cur_buffer += ReadBuffer(&buffer_[0], cur_buffer, size_); + buffer_.resize(static_cast<size_t>(file_size)); + + if (!buffer_.empty()) { + // std::vector does not define access to underlying array when empty + success = + ReadBuffer(&buffer_[0], &cur_buffer, buffer_.size(), &buffer_remaining); + if (!success) { + DLOG(ERROR) << "Buffer overrun"; + return -1; + } + } // Return the number of bytes read return static_cast<int>(std::distance(source, cur_buffer));
diff --git a/src/cobalt/storage/virtual_file.h b/src/cobalt/storage/virtual_file.h index 5741fd0..f5a9aec 100644 --- a/src/cobalt/storage/virtual_file.h +++ b/src/cobalt/storage/virtual_file.h
@@ -41,7 +41,7 @@ int Read(void* dest, int bytes, int offset) const; int Write(const void* source, int bytes, int offset); int Truncate(int size); - int Size() const { return size_; } + int Size() const { return static_cast<int>(buffer_.size()); } private: explicit VirtualFile(const std::string& name); @@ -49,10 +49,12 @@ // Returns the number of bytes written int Serialize(uint8* dest, const bool dry_run); - int Deserialize(const uint8* source); + // Deserializes a file, returning the size of the file or + // < 0 on error. + // |buffer_remaining| is the maximum size of |source| + int Deserialize(const uint8* source, size_t buffer_remaining); std::vector<uint8> buffer_; - int size_; std::string name_;
diff --git a/src/cobalt/storage/virtual_file_system.cc b/src/cobalt/storage/virtual_file_system.cc index 70e1556..9951df6 100644 --- a/src/cobalt/storage/virtual_file_system.cc +++ b/src/cobalt/storage/virtual_file_system.cc
@@ -14,7 +14,6 @@ * limitations under the License. */ - #include "cobalt/storage/virtual_file_system.h" #include "base/logging.h" @@ -124,23 +123,27 @@ return bytes_written; } -void VirtualFileSystem::Deserialize(const uint8* buffer, int buffer_size) { +bool VirtualFileSystem::Deserialize(const uint8* buffer, int buffer_size_in) { + const uint8* caller_buffer = buffer; + base::AutoLock lock(file_table_lock_); ClearFileTable(); - if (buffer_size < 0) { - DLOG(ERROR) << "Buffer size must be positive: " - << buffer_size << " < 0."; - return; + if (buffer_size_in < 0) { + DLOG(ERROR) << "Buffer size must be positive: " << buffer_size_in + << " < 0."; + return false; } + size_t buffer_size = static_cast<size_t>(buffer_size_in); + // The size of the buffer must be checked before copying the beginning of it // into a SerializedHeader. - if (static_cast<size_t>(buffer_size) < sizeof(SerializedHeader)) { + if (buffer_size < sizeof(SerializedHeader)) { DLOG(ERROR) << "Buffer size " << buffer_size << " is too small to contain a SerializedHeader of size " << sizeof(SerializedHeader) << "; operation aborted."; - return; + return false; } // Read in expected number of files @@ -152,23 +155,36 @@ if (buffer_version != 0) { // Note: We would handle old versions here, if necessary. DLOG(ERROR) << "Attempted to load a different version; operation aborted."; - return; - } else if (header.file_size != buffer_size) { + return false; + } else if (static_cast<size_t>(header.file_size) != buffer_size) { DLOG(ERROR) << "Buffer size mismatch: " << header.file_size << " != " << buffer_size; - return; + return false; } for (int i = 0; i < header.file_count; ++i) { + size_t buffer_used = static_cast<size_t>(buffer - caller_buffer); + if (buffer_size < buffer_used) { + DLOG(ERROR) << "Buffer overrun deserializing files"; + ClearFileTable(); + return false; + } + size_t buffer_remaining = buffer_size - buffer_used; + VirtualFile* file = new VirtualFile(""); - int bytes = file->Deserialize(buffer); + + int bytes = file->Deserialize(buffer, buffer_remaining); if (bytes > 0) { buffer += bytes; table_[file->name_] = file; } else { DLOG(WARNING) << "Failed to deserialize virtual file system."; + delete file; + ClearFileTable(); + return false; } } + return true; } void VirtualFileSystem::ClearFileTable() {
diff --git a/src/cobalt/storage/virtual_file_system.h b/src/cobalt/storage/virtual_file_system.h index e0b69b0..bef9a4c 100644 --- a/src/cobalt/storage/virtual_file_system.h +++ b/src/cobalt/storage/virtual_file_system.h
@@ -59,7 +59,8 @@ int Serialize(uint8* buffer, bool dry_run); // Deserializes a file system from a memory buffer. - void Deserialize(const uint8* buffer, int buffer_size); + // Returns false on failure. + bool Deserialize(const uint8* buffer, int buffer_size); // Simple file open. Will create a file if it does not exist, and files are // always readable and writable.
diff --git a/src/cobalt/storage/virtual_file_system_test.cc b/src/cobalt/storage/virtual_file_system_test.cc index e507516..de36f6f 100644 --- a/src/cobalt/storage/virtual_file_system_test.cc +++ b/src/cobalt/storage/virtual_file_system_test.cc
@@ -21,6 +21,8 @@ #include "base/memory/scoped_ptr.h" #include "testing/gtest/include/gtest/gtest.h" +#include "starboard/client_porting/poem/string_poem.h" + namespace cobalt { namespace storage { @@ -102,6 +104,32 @@ EXPECT_EQ(4, file->Size()); } +TEST_F(VirtualFileSystemTest, Truncate) { + VirtualFile* file = vfs_.Open("file1.tmp"); + ASSERT_TRUE(file); + EXPECT_EQ(0, file->Size()); + + const char* data = "test"; + char file_contents[4]; + // Write a few bytes of random data. + file->Write(data, 4, 0); + EXPECT_EQ(4, file->Size()); + int bytes = file->Read(file_contents, sizeof(file_contents), 0); + EXPECT_EQ(4, bytes); + EXPECT_EQ(0, memcmp(file_contents, data, static_cast<size_t>(bytes))); + + file->Truncate(3); + EXPECT_EQ(3, file->Size()); + bytes = file->Read(file_contents, sizeof(file_contents), 0); + EXPECT_EQ(3, bytes); + EXPECT_EQ(0, memcmp(file_contents, data, static_cast<size_t>(bytes))); + + file->Truncate(0); + EXPECT_EQ(0, file->Size()); + bytes = file->Read(file_contents, sizeof(file_contents), 0); + EXPECT_EQ(0, bytes); +} + TEST_F(VirtualFileSystemTest, SerializeDeserialize) { // Create a few files and write some data VirtualFile* file = vfs_.Open("file1.tmp"); @@ -116,6 +144,58 @@ int data2_size = 4; file->Write(data2, data2_size, 0); + file = vfs_.Open("file3.tmp"); + EXPECT_TRUE(file != NULL); + const char data3[] = ""; + int data3_size = 0; + file->Write(data3, data3_size, 0); + + // First perform a dry run to figure out how much space we need. + int bytes = vfs_.Serialize(NULL, true /*dry run*/); + scoped_array<uint8> buffer(new uint8[static_cast<size_t>(bytes)]); + + // Now serialize and deserialize + vfs_.Serialize(buffer.get(), false /*dry run*/); + + // Deserialize the data into a new vfs + VirtualFileSystem new_vfs; + ASSERT_TRUE(new_vfs.Deserialize(buffer.get(), bytes)); + + // Make sure the new vfs contains all the expected data. + char file_contents[VirtualFile::kMaxVfsPathname]; + file = new_vfs.Open("file1.tmp"); + EXPECT_TRUE(file != NULL); + bytes = file->Read(file_contents, sizeof(file_contents), 0); + EXPECT_EQ(data1_size, bytes); + EXPECT_EQ(0, memcmp(file_contents, data1, static_cast<size_t>(bytes))); + + file = new_vfs.Open("file2.tmp"); + EXPECT_TRUE(file != NULL); + bytes = file->Read(file_contents, sizeof(file_contents), 0); + EXPECT_EQ(data2_size, bytes); + EXPECT_EQ(0, memcmp(file_contents, data2, static_cast<size_t>(bytes))); + + file = new_vfs.Open("file3.tmp"); + EXPECT_TRUE(file != NULL); + bytes = file->Read(file_contents, sizeof(file_contents), 0); + EXPECT_EQ(data3_size, bytes); + EXPECT_EQ(0, memcmp(file_contents, data3, static_cast<size_t>(bytes))); +} + +TEST_F(VirtualFileSystemTest, DeserializeTruncated) { + // Create a few files and write some data + VirtualFile* file = vfs_.Open("file1.tmp"); + EXPECT_TRUE(file != NULL); + const char data1[] = "abc"; + int data1_size = 3; + file->Write(data1, data1_size, 0); + + file = vfs_.Open("file2.tmp"); + EXPECT_TRUE(file != NULL); + const char data2[] = "defg"; + int data2_size = 4; + file->Write(data2, data2_size, 0); + // First perform a dry run to figure out how much space we need. int bytes = vfs_.Serialize(NULL, true /*dry run*/); scoped_array<uint8> buffer(new uint8[static_cast<size_t>(bytes)]); @@ -123,23 +203,39 @@ // Now serialize and deserialize vfs_.Serialize(buffer.get(), false /*dry run*/); - // Deserialize the data into a new vfs - VirtualFileSystem new_vfs; - new_vfs.Deserialize(buffer.get(), bytes); + for (int i = 1; i < bytes; i++) { + // Corrupt the header + VirtualFileSystem::SerializedHeader header; + memcpy(&header, buffer.get(), sizeof(header)); + header.file_size = header.file_size - i; + memcpy(buffer.get(), &header, sizeof(header)); - // Make sure the new vfs contains all the expected data. - char file_contents[VirtualFile::kMaxVfsPathname]; - file = new_vfs.Open("file1.tmp"); - EXPECT_TRUE(file != NULL); - bytes = file->Read(file_contents, sizeof(file_contents), 0); - EXPECT_EQ(data1_size, bytes); - EXPECT_EQ(0, memcmp(file_contents, data1, static_cast<size_t>(bytes))); + // Deserialize the data into a new vfs + VirtualFileSystem new_vfs; + EXPECT_FALSE(new_vfs.Deserialize(buffer.get(), bytes - i)); + } +} - file = new_vfs.Open("file2.tmp"); - EXPECT_TRUE(file != NULL); - bytes = file->Read(file_contents, sizeof(file_contents), 0); - EXPECT_EQ(data2_size, bytes); - EXPECT_EQ(0, memcmp(file_contents, data2, static_cast<size_t>(bytes))); +TEST_F(VirtualFileSystemTest, DeserializeBadData) { + scoped_array<uint8> buffer(new uint8[0]); + EXPECT_EQ(false, vfs_.Deserialize(buffer.get(), 0)); + EXPECT_EQ(false, vfs_.Deserialize(buffer.get(), -1)); + buffer.reset(new uint8[1]); + EXPECT_EQ(false, vfs_.Deserialize(buffer.get(), 1)); + buffer.reset(new uint8[sizeof(VirtualFileSystem::SerializedHeader)]); + VirtualFileSystem::SerializedHeader header = {}; + header.version = -1; + memcpy(buffer.get(), &header, sizeof(header)); + EXPECT_EQ(false, + vfs_.Deserialize(buffer.get(), + sizeof(VirtualFileSystem::SerializedHeader))); + memcpy(&(header.version), "SAV0", sizeof(header.version)); + header.file_size = sizeof(VirtualFileSystem::SerializedHeader); + memcpy(buffer.get(), &header, sizeof(header)); + EXPECT_EQ(true, + vfs_.Deserialize(buffer.get(), + sizeof(VirtualFileSystem::SerializedHeader))); + ASSERT_EQ(0, vfs_.ListFiles().size()); } TEST_F(VirtualFileSystemTest, GetHeaderVersion) {