Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use-after-free on implicit temporary FileOptions #8571

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions db/column_family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ ColumnFamilyData::ColumnFamilyData(
uint32_t id, const std::string& name, Version* _dummy_versions,
Cache* _table_cache, WriteBufferManager* write_buffer_manager,
const ColumnFamilyOptions& cf_options, const ImmutableDBOptions& db_options,
const FileOptions& file_options, ColumnFamilySet* column_family_set,
const FileOptions* file_options, ColumnFamilySet* column_family_set,
BlockCacheTracer* const block_cache_tracer,
const std::shared_ptr<IOTracer>& io_tracer,
const std::string& db_session_id)
Expand Down Expand Up @@ -1462,14 +1462,14 @@ ColumnFamilySet::ColumnFamilySet(const std::string& dbname,
const std::shared_ptr<IOTracer>& io_tracer,
const std::string& db_session_id)
: max_column_family_(0),
file_options_(file_options),
dummy_cfd_(new ColumnFamilyData(
ColumnFamilyData::kDummyColumnFamilyDataId, "", nullptr, nullptr,
nullptr, ColumnFamilyOptions(), *db_options, file_options, nullptr,
nullptr, ColumnFamilyOptions(), *db_options, &file_options_, nullptr,
block_cache_tracer, io_tracer, db_session_id)),
default_cfd_cache_(nullptr),
db_name_(dbname),
db_options_(db_options),
file_options_(file_options),
table_cache_(table_cache),
write_buffer_manager_(_write_buffer_manager),
write_controller_(_write_controller),
Expand Down Expand Up @@ -1541,7 +1541,7 @@ ColumnFamilyData* ColumnFamilySet::CreateColumnFamily(
assert(column_families_.find(name) == column_families_.end());
ColumnFamilyData* new_cfd = new ColumnFamilyData(
id, name, dummy_versions, table_cache_, write_buffer_manager_, options,
*db_options_, file_options_, this, block_cache_tracer_, io_tracer_,
*db_options_, &file_options_, this, block_cache_tracer_, io_tracer_,
db_session_id_);
column_families_.insert({name, id});
column_family_data_.insert({id, new_cfd});
Expand Down
5 changes: 3 additions & 2 deletions db/column_family.h
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ class ColumnFamilyData {
WriteBufferManager* write_buffer_manager,
const ColumnFamilyOptions& options,
const ImmutableDBOptions& db_options,
const FileOptions& file_options,
const FileOptions* file_options,
ColumnFamilySet* column_family_set,
BlockCacheTracer* const block_cache_tracer,
const std::shared_ptr<IOTracer>& io_tracer,
Expand Down Expand Up @@ -722,6 +722,8 @@ class ColumnFamilySet {
std::unordered_map<uint32_t, ColumnFamilyData*> column_family_data_;

uint32_t max_column_family_;
const FileOptions file_options_;

ColumnFamilyData* dummy_cfd_;
// We don't hold the refcount here, since default column family always exists
// We are also not responsible for cleaning up default_cfd_cache_. This is
Expand All @@ -731,7 +733,6 @@ class ColumnFamilySet {

const std::string db_name_;
const ImmutableDBOptions* const db_options_;
const FileOptions file_options_;
Cache* table_cache_;
WriteBufferManager* write_buffer_manager_;
WriteController* write_controller_;
Expand Down
19 changes: 10 additions & 9 deletions db/repair.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class Repairer {
const ColumnFamilyOptions& unknown_cf_opts, bool create_unknown_cfs)
: dbname_(dbname),
env_(db_options.env),
env_options_(),
file_options_(),
db_options_(SanitizeOptions(dbname_, db_options)),
immutable_db_options_(ImmutableDBOptions(db_options_)),
icmp_(default_cf_opts.comparator),
Expand All @@ -112,14 +112,15 @@ class Repairer {
table_cache_(
// TODO: db_session_id for TableCache should be initialized after
// db_session_id_ is set.
new TableCache(default_iopts_, env_options_, raw_table_cache_.get(),
new TableCache(default_iopts_, &file_options_,
raw_table_cache_.get(),
/*block_cache_tracer=*/nullptr,
/*io_tracer=*/nullptr, /*db_session_id*/ "")),
wb_(db_options_.db_write_buffer_size),
wc_(db_options_.delayed_write_rate),
// TODO: db_session_id for VersionSet should be initialized after
// db_session_id_ is set and use it for initialization.
vset_(dbname_, &immutable_db_options_, env_options_,
vset_(dbname_, &immutable_db_options_, file_options_,
raw_table_cache_.get(), &wb_, &wc_,
/*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr,
/*db_session_id*/ ""),
Expand Down Expand Up @@ -249,7 +250,7 @@ class Repairer {
std::string const dbname_;
std::string db_session_id_;
Env* const env_;
const EnvOptions env_options_;
const FileOptions file_options_;
const DBOptions db_options_;
const ImmutableDBOptions immutable_db_options_;
const InternalKeyComparator icmp_;
Expand Down Expand Up @@ -366,7 +367,7 @@ class Repairer {
const auto& fs = env_->GetFileSystem();
std::unique_ptr<SequentialFileReader> lfile_reader;
Status status = SequentialFileReader::Create(
fs, logname, fs->OptimizeForLogRead(env_options_), &lfile_reader,
fs, logname, fs->OptimizeForLogRead(file_options_), &lfile_reader,
nullptr);
if (!status.ok()) {
return status;
Expand Down Expand Up @@ -458,7 +459,7 @@ class Repairer {
meta.fd.GetNumber());
status = BuildTable(
dbname_, /* versions */ nullptr, immutable_db_options_, tboptions,
env_options_, table_cache_.get(), iter.get(),
file_options_, table_cache_.get(), iter.get(),
std::move(range_del_iters), &meta, nullptr /* blob_file_additions */,
{}, kMaxSequenceNumber, snapshot_checker,
false /* paranoid_file_checks*/, nullptr /* internal_stats */, &io_s,
Expand Down Expand Up @@ -510,8 +511,8 @@ class Repairer {
file_size);
std::shared_ptr<const TableProperties> props;
if (status.ok()) {
status = table_cache_->GetTableProperties(env_options_, icmp_, t->meta.fd,
&props);
status = table_cache_->GetTableProperties(file_options_, icmp_,
t->meta.fd, &props);
}
if (status.ok()) {
t->column_family_id = static_cast<uint32_t>(props->column_family_id);
Expand Down Expand Up @@ -551,7 +552,7 @@ class Repairer {
ReadOptions ropts;
ropts.total_order_seek = true;
InternalIterator* iter = table_cache_->NewIterator(
ropts, env_options_, cfd->internal_comparator(), t->meta,
ropts, file_options_, cfd->internal_comparator(), t->meta,
nullptr /* range_del_agg */,
cfd->GetLatestMutableCFOptions()->prefix_extractor.get(),
/*table_reader_ptr=*/nullptr, /*file_read_hist=*/nullptr,
Expand Down
4 changes: 2 additions & 2 deletions db/table_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,12 @@ void AppendVarint64(IterKey* key, uint64_t v) {
const int kLoadConcurency = 128;

TableCache::TableCache(const ImmutableOptions& ioptions,
const FileOptions& file_options, Cache* const cache,
const FileOptions* file_options, Cache* const cache,
BlockCacheTracer* const block_cache_tracer,
const std::shared_ptr<IOTracer>& io_tracer,
const std::string& db_session_id)
: ioptions_(ioptions),
file_options_(file_options),
file_options_(*file_options),
cache_(cache),
immortal_tables_(false),
block_cache_tracer_(block_cache_tracer),
Expand Down
2 changes: 1 addition & 1 deletion db/table_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class HistogramImpl;
class TableCache {
public:
TableCache(const ImmutableOptions& ioptions,
const FileOptions& storage_options, Cache* cache,
const FileOptions* storage_options, Cache* cache,
BlockCacheTracer* const block_cache_tracer,
const std::shared_ptr<IOTracer>& io_tracer,
const std::string& db_session_id);
Expand Down