Skip to content

Commit

Permalink
[Fix](inverted index) fix point query buffer use after free
Browse files Browse the repository at this point in the history
  • Loading branch information
@airborne12
airborne12 committed Sep 12, 2023
1 parent df5839e commit 7c32b33
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 7 deletions.
4 changes: 2 additions & 2 deletions be/src/olap/comparison_predicate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,15 +213,15 @@ class ComparisonPredicateBase : public ColumnPredicate {
const Schema& schema) const override {
if (query_value == nullptr) {
auto column_desc = schema.column(_column_id);
if constexpr (PT == PredicateType::EQ) {
if constexpr (PT == PredicateType::EQ || PT == PredicateType::NE) {
query_value = std::make_unique<InvertedIndexPointQuery<Type, PT>>(
column_desc->type_info());
} else {
query_value = std::make_unique<InvertedIndexRangeQuery<Type, PredicateType::RANGE>>(
column_desc->type_info());
}
}
if constexpr (PT == PredicateType::EQ) {
if constexpr (PT == PredicateType::EQ || PT == PredicateType::NE) {
auto q = static_cast<InvertedIndexPointQuery<Type, PT>*>(query_value.get());
q->add_value(&_value, InvertedIndexQueryType::EQUAL_QUERY);
} else {
Expand Down
5 changes: 5 additions & 0 deletions be/src/olap/rowset/segment_v2/inverted_index/query/inverted_index_query.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,31 +170,36 @@ template <PrimitiveType Type, PredicateType PT>
Status InvertedIndexRangeQuery<Type, PT>::add_value(const T& value, InvertedIndexQueryType t) {
switch (t) {
case InvertedIndexQueryType::GREATER_THAN_QUERY: {
_low_value = &value;
_low_value_encoded.clear();
_value_key_coder->full_encode_ascending(&value, &_low_value_encoded);
break;
}

case InvertedIndexQueryType::GREATER_EQUAL_QUERY: {
_low_value = &value;
_inclusive_low = true;
_low_value_encoded.clear();
_value_key_coder->full_encode_ascending(&value, &_low_value_encoded);
break;
}

case InvertedIndexQueryType::LESS_THAN_QUERY: {
_high_value = &value;
_high_value_encoded.clear();
_value_key_coder->full_encode_ascending(&value, &_high_value_encoded);
break;
}

case InvertedIndexQueryType::LESS_EQUAL_QUERY: {
_high_value = &value;
_inclusive_high = true;
_high_value_encoded.clear();
_value_key_coder->full_encode_ascending(&value, &_high_value_encoded);
break;
}
case InvertedIndexQueryType::EQUAL_QUERY: {
_high_value = _low_value = &value;
_high_value_encoded.clear();
_value_key_coder->full_encode_ascending(&value, &_high_value_encoded);
_low_value_encoded.clear();
Expand Down
12 changes: 10 additions & 2 deletions be/src/olap/rowset/segment_v2/inverted_index/query/inverted_index_query.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class InvertedIndexPointQueryI : public InvertedIndexQueryBase {
LOG_FATAL("Execution reached an undefined behavior code path in InvertedIndexPointQueryI");
__builtin_unreachable();
}
[[nodiscard]] virtual std::vector<std::string> get_values() const {
[[nodiscard]] virtual const std::vector<std::string>& get_values() const {
LOG_FATAL("Execution reached an undefined behavior code path in InvertedIndexPointQueryI");
__builtin_unreachable();
};
Expand Down Expand Up @@ -122,7 +122,9 @@ class InvertedIndexPointQuery : public InvertedIndexPointQueryI {
}
return result;
}
[[nodiscard]] std::vector<std::string> get_values() const override { return _values_encoded; };
[[nodiscard]] const std::vector<std::string>& get_values() const override {
return _values_encoded;
};
[[nodiscard]] PredicateType get_predicate_type() const override { return PT; };
[[nodiscard]] InvertedIndexQueryType get_query_type() const override { return _type; };

Expand All @@ -146,6 +148,8 @@ class InvertedIndexRangeQueryI : public InvertedIndexQueryBase {
~InvertedIndexRangeQueryI() override = default;
[[nodiscard]] virtual const std::string& get_low_value() const = 0;
[[nodiscard]] virtual const std::string& get_high_value() const = 0;
virtual bool low_value_is_null() = 0;
virtual bool high_value_is_null() = 0;
QueryCategory get_query_category() override { return QueryCategory::RANGE_QUERY; }
std::string to_string() override {
LOG_FATAL("Execution reached an undefined behavior code path in InvertedIndexRangeQueryI");
Expand Down Expand Up @@ -208,6 +212,8 @@ class InvertedIndexRangeQuery : public InvertedIndexRangeQueryI {
[[nodiscard]] const std::string& get_high_value() const override {
return _high_value_encoded;
};
bool low_value_is_null() override { return _low_value == nullptr; };
bool high_value_is_null() override { return _high_value == nullptr; };
std::string to_string() override {
std::string low_op = _inclusive_low ? ">=" : ">";
std::string high_op = _inclusive_high ? "<=" : "<";
Expand All @@ -218,6 +224,8 @@ class InvertedIndexRangeQuery : public InvertedIndexRangeQueryI {
[[nodiscard]] bool is_high_value_inclusive() const override { return _inclusive_high; }

private:
const T* _low_value {};
const T* _high_value {};
std::string _low_value_encoded {};
std::string _high_value_encoded {};

Expand Down
11 changes: 8 additions & 3 deletions be/src/olap/rowset/segment_v2/inverted_index_reader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,13 +472,18 @@ Status StringTypeInvertedIndexReader::handle_range_query(const std::string& colu
nullptr, [](lucene::index::Term* term) { _CLDECDELETE(term); });
std::wstring column_name_ws = std::wstring(column_name.begin(), column_name.end());

if (query->low_value_is_null() && query->high_value_is_null()) {
return Status::Error<ErrorCode::INVERTED_INDEX_INVALID_PARAMETERS>(
"StringTypeInvertedIndexReader::handle_range_query error: both low_value and "
"high_value is null");
}
auto search_low = query->get_low_value();
if (!search_low.empty()) {
if (!query->low_value_is_null()) {
std::wstring search_low_ws = StringUtil::string_to_wstring(search_low);
low_term.reset(_CLNEW lucene::index::Term(column_name_ws.c_str(), search_low_ws.c_str()));
}
auto search_high = query->get_high_value();
if (!search_high.empty()) {
if (!query->high_value_is_null()) {
std::wstring search_high_ws = StringUtil::string_to_wstring(search_high);
high_term.reset(_CLNEW lucene::index::Term(column_name_ws.c_str(), search_high_ws.c_str()));
}
Expand Down Expand Up @@ -784,7 +789,7 @@ InvertedIndexVisitor::InvertedIndexVisitor(roaring::Roaring* h, InvertedIndexQue
}
} else if (query_value->get_query_category() == QueryCategory::POINT_QUERY) {
auto point_query = reinterpret_cast<InvertedIndexPointQueryI*>(query_value);
for (auto v : point_query->get_values()) {
for (const std::string& v : point_query->get_values()) {
query_points.emplace_back(v);
}
// =1 equals 1<= && >=1
Expand Down

0 comments on commit 7c32b33

Please sign in to comment.