From ec879982c223757d9c192ee7ea20deb1c890267e Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Fri, 16 Oct 2020 16:27:17 +0200 Subject: [PATCH] BUG/MEDIUM: lb: Always lock the server when calling server_{take,drop}_conn The server lock must be held when server_take_conn() and server_drop_conn() lbprm callback functions are called. It is a documented prerequisite but it is not always performed. It only affects leastconn and fas lb algorithm. Others don't use these callback functions. A race condition on the next pending effecive weight (next_eweight) may be encountered with the leastconn lb algorithm. An agent check may set it to 0 while fwlc_srv_reposition() is called. The server is locked during the next_eweight update. But because the server lock is not acquired when fwlc_srv_reposition() is called, we may use it to recompute the server key, leading to a division by 0. This patch must be backported as far as 1.8. (cherry picked from commit 26a52af642c3fcfd6a637aef019b78147c05e126) Signed-off-by: Christopher Faulet (cherry picked from commit e50dd9f3dd622937e0b7bb4cbe7067c50e9a1f13) Signed-off-by: Christopher Faulet --- src/backend.c | 5 ++++- src/stream.c | 10 ++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/backend.c b/src/backend.c index b0c1b08..e0b9cc5 100644 --- a/src/backend.c +++ b/src/backend.c @@ -1589,8 +1589,11 @@ int connect_server(struct stream *s) s->flags |= SF_CURR_SESS; count = _HA_ATOMIC_ADD(&srv->cur_sess, 1); HA_ATOMIC_UPDATE_MAX(&srv->counters.cur_sess_max, count); - if (s->be->lbprm.server_take_conn) + if (s->be->lbprm.server_take_conn) { + HA_SPIN_LOCK(SERVER_LOCK, &srv->lock); s->be->lbprm.server_take_conn(srv); + HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock); + } #ifdef USE_OPENSSL if (srv->ssl_ctx.sni) { diff --git a/src/stream.c b/src/stream.c index 6e07f37..ddf3b03 100644 --- a/src/stream.c +++ b/src/stream.c @@ -2994,8 +2994,11 @@ void sess_change_server(struct stream *sess, struct server *newsrv) _HA_ATOMIC_SUB(&sess->srv_conn->served, 1); _HA_ATOMIC_SUB(&sess->srv_conn->proxy->served, 1); __ha_barrier_atomic_store(); - if (sess->srv_conn->proxy->lbprm.server_drop_conn) + if (sess->srv_conn->proxy->lbprm.server_drop_conn) { + HA_SPIN_LOCK(SERVER_LOCK, &sess->srv_conn->lock); sess->srv_conn->proxy->lbprm.server_drop_conn(sess->srv_conn); + HA_SPIN_UNLOCK(SERVER_LOCK, &sess->srv_conn->lock); + } stream_del_srv_conn(sess); } @@ -3003,8 +3006,11 @@ void sess_change_server(struct stream *sess, struct server *newsrv) _HA_ATOMIC_ADD(&newsrv->served, 1); _HA_ATOMIC_ADD(&newsrv->proxy->served, 1); __ha_barrier_atomic_store(); - if (newsrv->proxy->lbprm.server_take_conn) + if (newsrv->proxy->lbprm.server_take_conn) { + HA_SPIN_LOCK(SERVER_LOCK, &newsrv->lock); newsrv->proxy->lbprm.server_take_conn(newsrv); + HA_SPIN_UNLOCK(SERVER_LOCK, &newsrv->lock); + } stream_add_srv_conn(sess, newsrv); } } -- 1.7.10.4