From c2f6e45aa3cc8b7a7f3fa217b11de83a7ac576f1 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Wed, 27 Sep 2017 23:57:07 -0700 Subject: [PATCH] prevent nullptr dereference in table reader error case Summary: A user encountered segfault on the call to `CacheDependencies()`, probably because `NewIndexIterator()` failed before populating `*index_entry`. Let's avoid the call in that case. Closes https://github.com/facebook/rocksdb/pull/2939 Differential Revision: D5928611 Pulled By: ajkr fbshipit-source-id: 484be453dbb00e5e160e9c6a1bc933df7d80f574 --- table/block_based_table_reader.cc | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc index d8c6d807c..78e90773f 100644 --- a/table/block_based_table_reader.cc +++ b/table/block_based_table_reader.cc @@ -813,15 +813,19 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions, CachableEntry index_entry; unique_ptr iter( new_table->NewIndexIterator(ReadOptions(), nullptr, &index_entry)); - index_entry.value->CacheDependencies(pin); - if (pin) { - rep->index_entry = std::move(index_entry); - } else { - index_entry.Release(table_options.block_cache.get()); - } s = iter->status(); - if (s.ok()) { + // This is the first call to NewIndexIterator() since we're in Open(). + // On success it should give us ownership of the `CachableEntry` by + // populating `index_entry`. + assert(index_entry.value != nullptr); + index_entry.value->CacheDependencies(pin); + if (pin) { + rep->index_entry = std::move(index_entry); + } else { + index_entry.Release(table_options.block_cache.get()); + } + // Hack: Call GetFilter() to implicitly add filter to the block_cache auto filter_entry = new_table->GetFilter(); if (filter_entry.value != nullptr) {