From: Willy Tarreau <w@1wt.eu>
Date: Thu, 14 Nov 2013 15:00:12 +0000 (+0100)
Subject: BUG/MINOR: acl: remove patterns from the tree before freeing them
X-Git-Tag: v1.5-dev20~247
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=60eccc18419763680f1cd08abfe8711e9170f9b0;p=thirdparty%2Fhaproxy.git

BUG/MINOR: acl: remove patterns from the tree before freeing them

A call to free_pattern_tree() upon exit() is made to free all ACL
patterns allocated in a tree (strings or IP addresses). Unfortunately
it happens that this function has been bogus from the beginning, it
walks over the whole tree, frees the nodes but forgets to remove them
from the tree prior to freeing them. So after visiting a leaf, the
next eb_next() call will require to revisit some of the upper nodes
that were just freed. This can remain unnoticed for a long time because
free() often just marks the area as free. But in cases of aggressive
memory freeing, the location will not be mapped anymore and the process
segfaults.

Note that the bug has no impact other than polluting kernel logs and
frightening sysadmins, since it happens just before exit().

Simply adding the debug code below makes it easier to reproduce the
same bug :

	while (node) {
		next = eb_next(node);
+		node->node_p = (void *)-1;
		free(node);
		node = next;
	}

Many thanks to the StackExchange team for their very detailed bug report
that permitted to quickly understand this non-obvious bug!

This fix should be backported to 1.4 which introduced the bug.
---

diff --git a/src/acl.c b/src/acl.c
index 0920a9ea68..af7d36b040 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -871,6 +871,7 @@ static void free_pattern_tree(struct eb_root *root)
 	node = eb_first(root);
 	while (node) {
 		next = eb_next(node);
+		eb_delete(node);
 		free(node);
 		node = next;
 	}