Fix asan failure caused by range tombstone start key use-after-free (#11106)

Summary:
the `last_tombstone_start_user_key` variable in `BuildTable()` and in `CompactionOutputs::AddRangeDels()` may point to a start key that is freed if user-defined timestamp is enabled. This was causing ASAN failure and this PR fixes this issue.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/11106

Test Plan: Added UT for repro.

Reviewed By: ajkr

Differential Revision: D42590862

Pulled By: cbi42

fbshipit-source-id: c493265ececdf89636d801d55ae929806c4d4b2c

diff --git a/db/builder.cc b/db/builder.cc
index 4cea24bfd436e1c7595bb25ff4b7b7416f78511f..a84bd5a45f469b5ba8742632fbcc25cdfc718719 100644 (file)
--- a/db/builder.cc
+++ b/db/builder.cc
@@ -253,7 +253,7 @@ Status BuildTable(
         if (version) {
           if (last_tombstone_start_user_key.empty() ||
               ucmp->CompareWithoutTimestamp(last_tombstone_start_user_key,
-                                            tombstone.start_key_) < 0) {
+                                            range_del_it->start_key()) < 0) {
             SizeApproximationOptions approx_opts;
             approx_opts.files_size_error_margin = 0.1;
             meta->compensated_range_deletion_size += versions->ApproximateSize(
@@ -261,7 +261,7 @@ Status BuildTable(
                 0 /* start_level */, -1 /* end_level */,
                 TableReaderCaller::kFlush);
           }
-          last_tombstone_start_user_key = tombstone.start_key_;
+          last_tombstone_start_user_key = range_del_it->start_key();
         }
       }
     }

diff --git a/db/compaction/compaction_outputs.cc b/db/compaction/compaction_outputs.cc
index 01333d774154aba327106a921fb21a2d540e062f..598bffb242f099d8b6dda4cb035a116ec4c196fb 100644 (file)
--- a/db/compaction/compaction_outputs.cc
+++ b/db/compaction/compaction_outputs.cc
@@ -616,8 +616,8 @@ Status CompactionOutputs::AddRangeDels(
       bool start_user_key_changed =
           last_tombstone_start_user_key.empty() ||
           ucmp->CompareWithoutTimestamp(last_tombstone_start_user_key,
-                                        tombstone.start_key_) < 0;
-      last_tombstone_start_user_key = tombstone.start_key_;
+                                        it->start_key()) < 0;
+      last_tombstone_start_user_key = it->start_key();
       // Range tombstones are truncated at file boundaries
       if (icmp.Compare(tombstone_start, meta.smallest) < 0) {
         tombstone_start = meta.smallest;

diff --git a/db/db_with_timestamp_basic_test.cc b/db/db_with_timestamp_basic_test.cc
index 6ea1aaf461baece3ce2063786fd08869a508c590..4208169236d189cf12b9dd397119fe20b19d6de7 100644 (file)
--- a/db/db_with_timestamp_basic_test.cc
+++ b/db/db_with_timestamp_basic_test.cc
@@ -3870,6 +3870,34 @@ TEST_F(DBBasicTestWithTimestamp, MergeAfterDeletion) {
 
   Close();
 }
+
+TEST_F(DBBasicTestWithTimestamp, RangeTombstoneApproximateSize) {
+  // Test code path for calculating range tombstone compensated size
+  // during flush and compaction.
+  Options options = CurrentOptions();
+  const size_t kTimestampSize = Timestamp(0, 0).size();
+  TestComparator test_cmp(kTimestampSize);
+  options.comparator = &test_cmp;
+  DestroyAndReopen(options);
+  // So that the compaction below is non-bottommost and will calcualte
+  // compensated range tombstone size.
+  ASSERT_OK(db_->Put(WriteOptions(), Key(1), Timestamp(1, 0), "val"));
+  ASSERT_OK(Flush());
+  MoveFilesToLevel(5);
+  ASSERT_OK(db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), Key(0),
+                             Key(1), Timestamp(1, 0)));
+  ASSERT_OK(db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), Key(1),
+                             Key(2), Timestamp(2, 0)));
+  ASSERT_OK(Flush());
+  ASSERT_OK(dbfull()->RunManualCompaction(
+      static_cast_with_check<ColumnFamilyHandleImpl>(db_->DefaultColumnFamily())
+          ->cfd(),
+      0 /* input_level */, 1 /* output_level */, CompactRangeOptions(),
+      nullptr /* begin */, nullptr /* end */, true /* exclusive */,
+      true /* disallow_trivial_move */,
+      std::numeric_limits<uint64_t>::max() /* max_file_num_to_ignore */,
+      "" /*trim_ts*/));
+}
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {