From bfd129cffebf23bfade54a927880dfb8dff2bb09 Mon Sep 17 00:00:00 2001 From: Tomasz Grabiec Date: Thu, 6 Aug 2020 16:54:17 +0200 Subject: [PATCH] thrift: Fix crash on unsorted column names in SlicePredicate The column names in SlicePredicate can be passed in arbitrary order. We converted them to clustering ranges in read_command preserving the original order. As a result, the clustering ranges in read command may appear out of order. This violates storage engine's assumptions and lead to undefined behavior. It was seen manifesting as a SIGSEGV or an abort in sstable reader when executing a get_slice() thrift verb: scylla: sstables/consumer.hh:476: seastar::future<> data_consumer::continuous_data_consumer::fast_forward_to(size_t, size_t) [with StateProcessor = sstables::data_consume_rows_context_m; size_t = long unsigned int]: Assertion `end >= _stream_position.position' failed. Fixes #6486. Tests: - added a new dtest to thrift_tests.py which reproduces the problem Message-Id: <1596725657-15802-1-git-send-email-tgrabiec@scylladb.com> --- thrift/handler.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/thrift/handler.cc b/thrift/handler.cc index dc0e77de6b..63594a4826 100644 --- a/thrift/handler.cc +++ b/thrift/handler.cc @@ -1471,6 +1471,15 @@ private: opts.set(query::partition_slice::option::send_partition_key); return opts; } + static void sort_ranges(const schema& s, std::vector& ranges) { + position_in_partition::less_compare less(s); + std::sort(ranges.begin(), ranges.end(), + [&less] (const query::clustering_range& r1, const query::clustering_range& r2) { + return less( + position_in_partition_view::for_range_start(r1), + position_in_partition_view::for_range_start(r2)); + }); + } static lw_shared_ptr slice_pred_to_read_cmd(service::storage_proxy& proxy, const schema& s, const SlicePredicate& predicate) { auto opts = query_opts(s); std::vector clustering_ranges; @@ -1484,6 +1493,7 @@ private: auto ckey = make_clustering_prefix(s, to_bytes(name)); clustering_ranges.emplace_back(query::clustering_range::make_singular(std::move(ckey))); } + sort_ranges(s, clustering_ranges); regular_columns.emplace_back(s.regular_begin()->id); } else { clustering_ranges.emplace_back(query::clustering_range::make_open_ended_both_sides());