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 <koponen@nicira.com>
CC: Teemu Koponen <koponen@nicira.com>

diff --git a/lib/classifier.c b/lib/classifier.c
index 591322e7748fc5598c2ae82ab882b2b5fe0d3cba..3842422974a069609dd8efab26267b5e5ad711ce 100644 (file)
--- 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 9af97a4c853b1c929e22b40a23389c24efaf53b3..f4ccfdfe693785875f9222f2f7f6fe1a8c6b0291 100644 (file)
--- 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 *