From 955f579d42ff86c5cac4c3913b1cf42c27ebcc04 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 19 Nov 2010 16:41:02 -0800 Subject: [PATCH] classifier: Fix segfault iterating with rules that differ only in priority. When CLS_CURSOR_FOR_EACH(_SAFE) iterated through a classifier, the cls_cursor_next() function did not properly handle the case where there was more than a single rule on a list. This commit fixes the problem. The addition to the testsuite would have found the problem earlier. Reported-by: Teemu Koponen CC: Teemu Koponen --- lib/classifier.c | 18 +++++++++++++++--- tests/test-classifier.c | 9 +++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/classifier.c b/lib/classifier.c index 591322e7..38424229 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -52,6 +52,7 @@ static void zero_wildcards(struct flow *, const struct flow_wildcards *); (RULE) != NULL && ((NEXT) = next_rule_in_list(RULE), true); \ (RULE) = (NEXT)) +static struct cls_rule *next_rule_in_list__(struct cls_rule *); static struct cls_rule *next_rule_in_list(struct cls_rule *); static struct cls_table * @@ -601,11 +602,15 @@ cls_cursor_next(struct cls_cursor *cursor, struct cls_rule *rule) const struct cls_table *table; struct cls_rule *next; - next = next_rule_in_list(rule); - if (next) { + next = next_rule_in_list__(rule); + if (next->priority < rule->priority) { return next; } + /* 'next' is the head of the list, that is, the rule that is included in + * the table's hmap. (This is important when the classifier contains rules + * that differ only in priority.) */ + rule = next; HMAP_FOR_EACH_CONTINUE (rule, hmap_node, &cursor->table->rules) { if (rule_matches(rule, cursor->target)) { return rule; @@ -744,9 +749,16 @@ insert_rule(struct cls_table *table, struct cls_rule *new) } static struct cls_rule * -next_rule_in_list(struct cls_rule *rule) +next_rule_in_list__(struct cls_rule *rule) { struct cls_rule *next = OBJECT_CONTAINING(rule->list.next, next, list); + return next; +} + +static struct cls_rule * +next_rule_in_list(struct cls_rule *rule) +{ + struct cls_rule *next = next_rule_in_list__(rule); return next->priority < rule->priority ? next : NULL; } diff --git a/tests/test-classifier.c b/tests/test-classifier.c index 9af97a4c..f4ccfdfe 100644 --- a/tests/test-classifier.c +++ b/tests/test-classifier.c @@ -413,9 +413,12 @@ check_tables(const struct classifier *cls, { const struct cls_table *table; struct flow_wildcards exact_wc; + struct test_rule *test_rule; + struct cls_cursor cursor; int found_tables = 0; int found_rules = 0; int found_dups = 0; + int found_rules2 = 0; flow_wildcards_init_exact(&exact_wc); HMAP_FOR_EACH (table, hmap_node, &cls->tables) { @@ -443,6 +446,12 @@ check_tables(const struct classifier *cls, assert(n_tables == -1 || n_tables == hmap_count(&cls->tables)); assert(n_rules == -1 || found_rules == n_rules); assert(n_dups == -1 || found_dups == n_dups); + + cls_cursor_init(&cursor, cls, NULL); + CLS_CURSOR_FOR_EACH (test_rule, cls_rule, &cursor) { + found_rules2++; + } + assert(found_rules == found_rules2); } static struct test_rule * -- 2.30.2