From bff005ae587c8d51deb4a1e916a2b9e42f0c6eb1 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 27 May 2019 08:10:11 +0200 Subject: [PATCH] BUG/MEDIUM: queue: fix the tree walk in pendconn_redistribute. In pendconn_redistribute() we scan the queue using eb32_next() on the node we've just deleted, which is wrong since the node is not in the tree anymore, and it could dereference one node that has already been released by another thread. Note that we cannot use eb32_first() in the loop here instead because we need to skip pendconns having SF_FORCE_PRST. Instead, let's keep a copy of the next node before deleting it. In addition, the pendconn retrieved there is wrong, it uses &node as the pointer instead of node, resulting in very quick crashes when the server list is scanned. Fortunately this only happens when "option redispatch" is used in conjunction with "maxconn" on server lines, "cookie" for the stickiness, and when a server goes down with entries in its queue. This bug was introduced by commit 0355dabd7 ("MINOR: queue: replace the linked list with a tree") so the fix must be backported to 1.9. --- src/queue.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/queue.c b/src/queue.c index e4703df6a..f4a945301 100644 --- a/src/queue.c +++ b/src/queue.c @@ -397,7 +397,7 @@ struct pendconn *pendconn_add(struct stream *strm) int pendconn_redistribute(struct server *s) { struct pendconn *p; - struct eb32_node *node; + struct eb32_node *node, *nodeb; int xferred = 0; /* The REDISP option was specified. We will ignore cookie and force to @@ -405,8 +405,10 @@ int pendconn_redistribute(struct server *s) if ((s->proxy->options & (PR_O_REDISP|PR_O_PERSIST)) != PR_O_REDISP) return 0; - for (node = eb32_first(&s->pendconns); node; node = eb32_next(node)) { - p = eb32_entry(&node, struct pendconn, node); + for (node = eb32_first(&s->pendconns); node; node = nodeb) { + nodeb = eb32_next(node); + + p = eb32_entry(node, struct pendconn, node); if (p->strm_flags & SF_FORCE_PRST) continue;