From: Willy Tarreau <w@1wt.eu>
Date: Sat, 9 Jun 2012 13:43:36 +0000 (+0200)
Subject: BUG/MEDIUM: ebtree: ebmb_insert() must not call cmp_bits on full-length matches
X-Git-Tag: v1.5-dev12~144
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a4a1cd107223ff275b83d956430b64fdb9594f4c;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: ebtree: ebmb_insert() must not call cmp_bits on full-length matches

Otherwise we end up comparing the byte past the end, resulting
in duplicate values still being inserted into the tree even if
undesired.

This generally has low impact, though it can sometimes cause one new entry
to be added next to an existing one for stick tables, preventing the results
from being merged.

(cherry picked from commit 12e54ac493a91bb02064568f410592c2700d3933)
---

diff --git a/ebtree/ebimtree.h b/ebtree/ebimtree.h
index b40e490b4e..1fe9fa0acb 100644
--- a/ebtree/ebimtree.h
+++ b/ebtree/ebimtree.h
@@ -198,7 +198,14 @@ __ebim_insert(struct eb_root *root, struct ebpt_node *new, unsigned int len)
 			 * The last two cases can easily be partially merged.
 			 */
 			bit = equal_bits(new->key, old->key, bit, len);
-			diff = cmp_bits(new->key, old->key, bit);
+
+			/* Note: we can compare more bits than the current node's because as
+			 * long as they are identical, we know we descend along the correct
+			 * side. However we don't want to start to compare past the end.
+			 */
+			diff = 0;
+			if (((unsigned)bit >> 3) < len)
+				diff = cmp_bits(new->key, old->key, bit);
 
 			if (diff < 0) {
 				new->node.leaf_p = new_left;
@@ -263,7 +270,14 @@ __ebim_insert(struct eb_root *root, struct ebpt_node *new, unsigned int len)
 
 			new->node.node_p = old->node.node_p;
 
-			diff = cmp_bits(new->key, old->key, bit);
+			/* Note: we can compare more bits than the current node's because as
+			 * long as they are identical, we know we descend along the correct
+			 * side. However we don't want to start to compare past the end.
+			 */
+			diff = 0;
+			if (((unsigned)bit >> 3) < len)
+				diff = cmp_bits(new->key, old->key, bit);
+
 			if (diff < 0) {
 				new->node.leaf_p = new_left;
 				old->node.node_p = new_rght;
diff --git a/ebtree/ebmbtree.h b/ebtree/ebmbtree.h
index 121c59fa12..fa25ac58aa 100644
--- a/ebtree/ebmbtree.h
+++ b/ebtree/ebmbtree.h
@@ -306,12 +306,16 @@ __ebmb_insert(struct eb_root *root, struct ebmb_node *new, unsigned int len)
 	new_rght = eb_dotag(&new->node.branches, EB_RGHT);
 	new_leaf = eb_dotag(&new->node.branches, EB_LEAF);
 
-	/* Note: we can compare more bits than
-	 * the current node's because as long as they are identical, we
-	 * know we descend along the correct side.
-	 */
 	new->node.bit = bit;
-	diff = cmp_bits(new->key, old->key, bit);
+
+	/* Note: we can compare more bits than the current node's because as
+	 * long as they are identical, we know we descend along the correct
+	 * side. However we don't want to start to compare past the end.
+	 */
+	diff = 0;
+	if (((unsigned)bit >> 3) < len)
+		diff = cmp_bits(new->key, old->key, bit);
+
 	if (diff == 0) {
 		new->node.bit = -1; /* mark as new dup tree, just in case */