Skip to content

Commit

Permalink
thrift: Fix crash on unsorted column names in SlicePredicate
Browse files Browse the repository at this point in the history
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>
(cherry picked from commit bfd129c)
  • Loading branch information
tgrabiec authored and avikivity committed Aug 8, 2020
1 parent da9e708 commit 4035cf4
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions thrift/handler.cc
Expand Up @@ -1466,6 +1466,15 @@ class thrift_handler : public CassandraCobSvIf {
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(const schema& s, const SlicePredicate& predicate) {
auto opts = query_opts(s);
std::vector<query::clustering_range> clustering_ranges;
Expand All @@ -1479,6 +1488,7 @@ class thrift_handler : public CassandraCobSvIf {
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());
Expand Down

0 comments on commit 4035cf4

Please sign in to comment.