From 469a9f32a7583b1650e904b8e5d603dc81f762b7 Mon Sep 17 00:00:00 2001
From: Igor Canadi <icanadi@fb.com>
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,
-- 
GitLab