From fa8c050e9ff2fa2e25e3620798f0d60c3c72157e Mon Sep 17 00:00:00 2001 From: Rohan Rathi Date: Thu, 22 Mar 2018 14:04:06 -0700 Subject: [PATCH] Fixed buffer overrun in BackupEngineImpl::BackupMeta::StoreToFile Summary: The 10MB buffer in BackupEngineImpl::BackupMeta::StoreToFile can be corrupted with a large number of files. Added a check to determine current buffer length and append data to file if buffer becomes full. Resolves https://github.com/facebook/rocksdb/issues/3228 Closes https://github.com/facebook/rocksdb/pull/3636 Differential Revision: D7354160 Pulled By: ajkr fbshipit-source-id: eec12d38095a0d17551a4aaee52b99d30a555722 --- utilities/backupable/backupable_db.cc | 50 ++++++++++++++++++++------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/utilities/backupable/backupable_db.cc b/utilities/backupable/backupable_db.cc index bccb7efd2..15a7bfb53 100644 --- a/utilities/backupable/backupable_db.cc +++ b/utilities/backupable/backupable_db.cc @@ -1733,25 +1733,51 @@ Status BackupEngineImpl::BackupMeta::StoreToFile(bool sync) { if (!app_metadata_.empty()) { std::string hex_encoded_metadata = Slice(app_metadata_).ToString(/* hex */ true); + + // +1 to accomodate newline character + size_t hex_meta_strlen = kMetaDataPrefix.ToString().length() + hex_encoded_metadata.length() + 1; + if (hex_meta_strlen >= buf_size) { + return Status::Corruption("Buffer too small to fit backup metadata"); + } + else if (len + hex_meta_strlen >= buf_size) { + backup_meta_file->Append(Slice(buf.get(), len)); + buf.reset(); + unique_ptr new_reset_buf(new char[max_backup_meta_file_size_]); + buf.swap(new_reset_buf); + len = 0; + } len += snprintf(buf.get() + len, buf_size - len, "%s%s\n", kMetaDataPrefix.ToString().c_str(), hex_encoded_metadata.c_str()); - if (len >= buf_size) { - return Status::Corruption("Buffer too small to fit backup metadata"); - } } - len += snprintf(buf.get() + len, buf_size - len, "%" ROCKSDB_PRIszt "\n", - files_.size()); - if (len >= buf_size) { - return Status::Corruption("Buffer too small to fit backup metadata"); + + char writelen_temp[19]; + if (len + sprintf(writelen_temp, "%" ROCKSDB_PRIszt "\n", files_.size()) >= buf_size) { + backup_meta_file->Append(Slice(buf.get(), len)); + buf.reset(); + unique_ptr new_reset_buf(new char[max_backup_meta_file_size_]); + buf.swap(new_reset_buf); + len = 0; } + { + const char *const_write = writelen_temp; + len += snprintf(buf.get() + len, buf_size - len, "%s", const_write); + } + for (const auto& file : files_) { // use crc32 for now, switch to something else if needed - len += snprintf(buf.get() + len, buf_size - len, "%s crc32 %u\n", - file->filename.c_str(), file->checksum_value); - if (len >= buf_size) { - return Status::Corruption("Buffer too small to fit backup metadata"); - } + + size_t newlen = len + file->filename.length() + sprintf(writelen_temp, " crc32 %u\n", file->checksum_value); + const char *const_write = writelen_temp; + if (newlen >= buf_size) { + backup_meta_file->Append(Slice(buf.get(), len)); + buf.reset(); + unique_ptr new_reset_buf(new char[max_backup_meta_file_size_]); + buf.swap(new_reset_buf); + len = 0; + } + len += snprintf(buf.get() + len, buf_size - len, "%s%s", + file->filename.c_str(), const_write); } s = backup_meta_file->Append(Slice(buf.get(), len));