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) {
