]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: pattern: fix possible infinite loops on deletion (try 2) master
authorWilly Tarreau <w@1wt.eu>
Tue, 16 Sep 2025 09:49:01 +0000 (11:49 +0200)
committerWilly Tarreau <w@1wt.eu>
Tue, 16 Sep 2025 14:32:39 +0000 (16:32 +0200)
Commit e36b3b60b3 ("MEDIUM: migrate the patterns reference to cebs_tree")
changed the construction of the loops used to look up matching nodes, and
since we don't need two elements anymore, the "continue" statement now
loops on the same element when deleting. Let's fix this to make sure it
passes through the next one.

While this bug is 3.3 only, it turns out that 3.2 is also affected by
the incorrect loop construct in pat_ref_set_from_node(), where it's
possible to run an infinite loop since commit 010c34b8c7 ("MEDIUM:
pattern: consider gen_id in pat_ref_set_from_node()") due to the
"continue" statement being placed before the ebmb_next_dup() call.

As such the relevant part of this fix (pat_ref_set_from_elt) will
need to be backported to 3.2.

src/pattern.c

index 9971841394c46e843aacdf2a67aec86713af94a2..04a76d207efedf76204e35702d6ab727d04f0c2f 100644 (file)
@@ -1626,18 +1626,18 @@ int pat_ref_delete_by_id(struct pat_ref *ref, struct pat_ref_elt *refelt)
  */
 int pat_ref_gen_delete(struct pat_ref *ref, unsigned int gen_id, const char *key)
 {
-       struct pat_ref_elt *elt, *next;
+       struct pat_ref_elt *elt, *elt2;
        int found = 0;
 
        /* delete pattern from reference */
        elt = cebs_item_lookup(&ref->ceb_root, node, pattern, key, struct pat_ref_elt);
        while (elt) {
-               if (elt->gen_id != gen_id)
-                       continue;
-               next = cebs_item_next_dup(&ref->ceb_root, node, pattern, elt);
-               pat_ref_delete_by_ptr(ref, elt);
-               found = 1;
-               elt = next;
+               elt2 = cebs_item_next_dup(&ref->ceb_root, node, pattern, elt);
+               if (elt->gen_id == gen_id) {
+                       pat_ref_delete_by_ptr(ref, elt);
+                       found = 1;
+               }
+               elt = elt2;
        }
 
        if (found)
@@ -1787,12 +1787,14 @@ int pat_ref_set_by_id(struct pat_ref *ref, struct pat_ref_elt *refelt, const cha
 static int pat_ref_set_from_elt(struct pat_ref *ref, struct pat_ref_elt *elt, const char *value, char **err)
 {
        unsigned int gen;
+       struct pat_ref_elt *elt2;
        int first = 1;
        int found = 0;
 
-       while (elt) {
+       for (; elt; elt = elt2) {
                char *tmp_err = NULL;
 
+               elt2 = cebs_item_next_dup(&ref->ceb_root, node, pattern, elt);
                if (first)
                        gen = elt->gen_id;
                else if (elt->gen_id != gen) {
@@ -1809,7 +1811,6 @@ static int pat_ref_set_from_elt(struct pat_ref *ref, struct pat_ref_elt *elt, co
                }
                found = 1;
                first = 0;
-               elt = cebs_item_next_dup(&ref->ceb_root, node, pattern, elt);
        }
 
        if (!found) {