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<StateProcessor>::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>
This commit is contained in:
Tomasz Grabiec
2020-08-06 16:54:17 +02:00
committed by Avi Kivity
parent b1315a2120
commit bfd129cffe

View File

@@ -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<query::clustering_range>& 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<query::read_command> slice_pred_to_read_cmd(service::storage_proxy& proxy, const schema& s, const SlicePredicate& predicate) {
auto opts = query_opts(s);
std::vector<query::clustering_range> 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());