Skip to content

Commit

Permalink
Order rule traversal in RuleMatcher matching operations by Rule index
Browse files Browse the repository at this point in the history
This ordering fixes a test failure we're seeing on Alpine for the
signatures/tcp-end-of-match btest, since discrepancies in rule match traversal
could lead to discrepancies in corresponding event ordering. It looks safe to
rely on across platforms since the index is driven by signature load order,
which shouldn't deviate. If this somehow doesn't hold in the future, we'll only
wind up with a test failure, not incorrect match behavior.
  • Loading branch information
ckreibich committed Nov 18, 2024
1 parent 62bc324 commit eed73b7
Showing 1 changed file with 8 additions and 9 deletions.
17 changes: 8 additions & 9 deletions src/RuleMatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -657,19 +657,19 @@ RuleMatcher::MIME_Matches* RuleMatcher::Match(RuleFileMagicState* state, const u
}

// Find rules for which patterns have matched.
set<Rule*> rule_matches;
map<decltype(Rule::idx), Rule*> rule_matches;

for ( AcceptingMatchSet::const_iterator it = accepted_matches.begin(); it != accepted_matches.end(); ++it ) {
auto [aidx, mpos] = *it;

Rule* r = Rule::rule_table[aidx - 1];

if ( AllRulePatternsMatched(r, mpos, accepted_matches) )
rule_matches.insert(r);
rule_matches[r->Index()] = r;
}

for ( set<Rule*>::const_iterator it = rule_matches.begin(); it != rule_matches.end(); ++it ) {
Rule* r = *it;
for ( const auto& entry : rule_matches ) {
Rule* r = entry.second;

for ( const auto& action : r->actions ) {
const RuleActionMIME* ram = dynamic_cast<const RuleActionMIME*>(action);
Expand Down Expand Up @@ -841,7 +841,7 @@ void RuleMatcher::Match(RuleEndpointState* state, Rule::PatternType type, const
// matched patterns per connection (which is a plausible assumption).

// Find rules for which patterns have matched.
set<pair<Rule*, MatchPos>> rule_matches;
map<decltype(Rule::idx), std::pair<Rule*, MatchPos>> rule_matches;

for ( AcceptingMatchSet::const_iterator it = accepted_matches.begin(); it != accepted_matches.end(); ++it ) {
AcceptIdx aidx = it->first;
Expand All @@ -850,13 +850,12 @@ void RuleMatcher::Match(RuleEndpointState* state, Rule::PatternType type, const
Rule* r = Rule::rule_table[aidx - 1];

if ( AllRulePatternsMatched(r, mpos, accepted_matches) )
rule_matches.insert(make_pair(r, mpos));
rule_matches[r->Index()] = make_pair(r, mpos);
}

// Check which of the matching rules really belong to any of our nodes.

for ( set<pair<Rule*, MatchPos>>::const_iterator it = rule_matches.begin(); it != rule_matches.end(); ++it ) {
auto [r, match_end_pos] = *it;
for ( const auto& entry : rule_matches ) {
auto [r, match_end_pos] = entry.second;

DBG_LOG(DBG_RULES, "Accepted rule: %s", r->id);

Expand Down

0 comments on commit eed73b7

Please sign in to comment.