From 54f9064a3286ab45872640ad19bc84efc719179f Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 22 Sep 2021 07:15:57 +0200 Subject: [PATCH] BUG/MEDIUM: leastconn: fix rare possibility of divide by zero An optimization was brought in commit 5064ab6a9 ("OPTIM: lb-leastconn: do not unlink the server if it did not change") to avoid locking the server just to discover it did not move. However a mistake was made because the operation involves a divide with a value that is read outside of its usual lock, which makes it possible to be zero at the exact moment we watch it if another thread takes the server down under the lbprm lock, resulting in a divide by zero. Therefore we must check that the value is not null there. This must be backported to 2.4. (cherry picked from commit 6f97b4ef3383f59ff3218f73fcd21f94335b2c1b) Signed-off-by: Christopher Faulet (cherry picked from commit 8031464835df8166666f315515604fe6bc3c5598) [cf: Above commit was backported to 2.3. Thus this patch should be backported too.] Signed-off-by: Christopher Faulet --- src/lb_fwlc.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lb_fwlc.c b/src/lb_fwlc.c index 4e1b730..9f310ac 100644 --- a/src/lb_fwlc.c +++ b/src/lb_fwlc.c @@ -73,12 +73,13 @@ static inline void fwlc_queue_srv(struct server *s, unsigned int eweight) static void fwlc_srv_reposition(struct server *s, int locked) { unsigned int inflight = _HA_ATOMIC_LOAD(&s->served) + _HA_ATOMIC_LOAD(&s->nbpend); - unsigned int new_key = inflight ? (inflight + 1) * SRV_EWGHT_MAX / s->cur_eweight : 0; + unsigned int eweight = _HA_ATOMIC_LOAD(&s->cur_eweight); + unsigned int new_key = inflight ? (inflight + 1) * SRV_EWGHT_MAX / (eweight ? eweight : 1) : 0; /* some calls will be made for no change (e.g connect_server() after * assign_server(). Let's check that first. */ - if (s->lb_node.node.leaf_p && s->lb_node.key == new_key) + if (s->lb_node.node.leaf_p && eweight && s->lb_node.key == new_key) return; HA_RWLOCK_WRLOCK(LBPRM_LOCK, &s->proxy->lbprm.lock); @@ -89,7 +90,8 @@ static void fwlc_srv_reposition(struct server *s, int locked) * to our target value (50% of the case in measurements). */ inflight = _HA_ATOMIC_LOAD(&s->served) + _HA_ATOMIC_LOAD(&s->nbpend); - new_key = inflight ? (inflight + 1) * SRV_EWGHT_MAX / s->cur_eweight : 0; + eweight = _HA_ATOMIC_LOAD(&s->cur_eweight); + new_key = inflight ? (inflight + 1) * SRV_EWGHT_MAX / (eweight ? eweight : 1) : 0; if (!s->lb_node.node.leaf_p || s->lb_node.key != new_key) { eb32_delete(&s->lb_node); s->lb_node.key = new_key; -- 1.7.10.4