From 469a9f32a7583b1650e904b8e5d603dc81f762b7 Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Tue, 19 Nov 2013 21:01:48 -0800 Subject: [PATCH] Fix two nasty use-after-free-bugs Summary: These bugs were caught by ASAN crash test. 1. The first one, in table/filter_block.cc is very nasty. We first reference entries_ and store the reference to Slice prev. Then, we call entries_.append(), which can change the reference. The Slice prev now points to junk. 2. The second one is a bug in a test, so it's not very serious. Once we set read_opts.prefix, we never clear it, so some other function might still reference it. Test Plan: asan crash test now runs more than 5 mins. Before, it failed immediately. I will run the full one, but the full one takes quite some time (5 hours) Reviewers: dhruba, haobo, kailiu Reviewed By: dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D14223 --- table/filter_block.cc | 15 +++++++++------ tools/db_stress.cc | 5 +++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/table/filter_block.cc b/table/filter_block.cc index 98030a4c3..82b6c6ee1 100644 --- a/table/filter_block.cc +++ b/table/filter_block.cc @@ -52,19 +52,22 @@ bool FilterBlockBuilder::SamePrefix(const Slice &key1, void FilterBlockBuilder::AddKey(const Slice& key) { // get slice for most recently added entry Slice prev; - if (start_.size() > 0) { - size_t prev_start = start_[start_.size() - 1]; - const char* base = entries_.data() + prev_start; - size_t length = entries_.size() - prev_start; - prev = Slice(base, length); - } + size_t added_to_start = 0; // add key to filter if needed if (whole_key_filtering_) { start_.push_back(entries_.size()); + ++added_to_start; entries_.append(key.data(), key.size()); } + if (start_.size() > added_to_start) { + size_t prev_start = start_[start_.size() - 1 - added_to_start]; + const char* base = entries_.data() + prev_start; + size_t length = entries_.size() - prev_start; + prev = Slice(base, length); + } + // add prefix to filter if needed if (prefix_extractor_ && prefix_extractor_->InDomain(ExtractUserKey(key))) { // If prefix_extractor_, this filter_block layer assumes we only diff --git a/tools/db_stress.cc b/tools/db_stress.cc index da6fa7f2c..71e36e901 100644 --- a/tools/db_stress.cc +++ b/tools/db_stress.cc @@ -1118,6 +1118,7 @@ class StressTest { } else { MultiPrefixScan(thread, read_opts, prefix); } + read_opts.prefix = nullptr; } else if (prefixBound <= prob_op && prob_op < writeBound) { // OPERATION write uint32_t value_base = thread->rand.Next(); @@ -1126,8 +1127,8 @@ class StressTest { if (!FLAGS_test_batches_snapshots) { MutexLock l(thread->shared->GetMutexForKey(rand_key)); if (FLAGS_verify_before_write) { - std::string keystr = Key(rand_key); - Slice k = keystr; + std::string keystr2 = Key(rand_key); + Slice k = keystr2; Status s = db_->Get(read_opts, k, &from_db); VerifyValue(rand_key, read_opts,