commit 0dbaa252df906cc9c1d0dc7a075c16e039ab1c5b Author: Willy Tarreau Date: Tue Aug 21 15:35:31 2018 +0200 BUG/MEDIUM: cli/threads: protect some server commands against concurrent operations The server-specific CLI commands "set weight", "set maxconn", "disable agent", "enable agent", "disable health", "enable health", "disable server" and "enable server" were not protected against concurrent accesses. Now they take the server lock around the sensitive part. This patch must be backported to 1.8. (cherry picked from commit 3bcc2699ba08dd3971ae7a56631994b2524d2acb) Signed-off-by: Willy Tarreau diff --git a/src/server.c b/src/server.c index 36a05e27..98dae535 100644 --- a/src/server.c +++ b/src/server.c @@ -4299,6 +4299,10 @@ static int cli_parse_get_weight(char **args, struct appctx *appctx, void *privat return 1; } +/* Parse a "set weight" command. + * + * Grabs the server lock. + */ static int cli_parse_set_weight(char **args, struct appctx *appctx, void *private) { struct server *sv; @@ -4311,16 +4315,24 @@ static int cli_parse_set_weight(char **args, struct appctx *appctx, void *privat if (!sv) return 1; + HA_SPIN_LOCK(SERVER_LOCK, &sv->lock); + warning = server_parse_weight_change_request(sv, args[3]); if (warning) { appctx->ctx.cli.severity = LOG_ERR; appctx->ctx.cli.msg = warning; appctx->st0 = CLI_ST_PRINT; } + + HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock); + return 1; } -/* parse a "set maxconn server" command. It always returns 1. */ +/* parse a "set maxconn server" command. It always returns 1. + * + * Grabs the server lock. + */ static int cli_parse_set_maxconn_server(char **args, struct appctx *appctx, void *private) { struct server *sv; @@ -4333,16 +4345,24 @@ static int cli_parse_set_maxconn_server(char **args, struct appctx *appctx, void if (!sv) return 1; + HA_SPIN_LOCK(SERVER_LOCK, &sv->lock); + warning = server_parse_maxconn_change_request(sv, args[4]); if (warning) { appctx->ctx.cli.severity = LOG_ERR; appctx->ctx.cli.msg = warning; appctx->st0 = CLI_ST_PRINT; } + + HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock); + return 1; } -/* parse a "disable agent" command. It always returns 1. */ +/* parse a "disable agent" command. It always returns 1. + * + * Grabs the server lock. + */ static int cli_parse_disable_agent(char **args, struct appctx *appctx, void *private) { struct server *sv; @@ -4354,11 +4374,16 @@ static int cli_parse_disable_agent(char **args, struct appctx *appctx, void *pri if (!sv) return 1; + HA_SPIN_LOCK(SERVER_LOCK, &sv->lock); sv->agent.state &= ~CHK_ST_ENABLED; + HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock); return 1; } -/* parse a "disable health" command. It always returns 1. */ +/* parse a "disable health" command. It always returns 1. + * + * Grabs the server lock. + */ static int cli_parse_disable_health(char **args, struct appctx *appctx, void *private) { struct server *sv; @@ -4370,11 +4395,16 @@ static int cli_parse_disable_health(char **args, struct appctx *appctx, void *pr if (!sv) return 1; + HA_SPIN_LOCK(SERVER_LOCK, &sv->lock); sv->check.state &= ~CHK_ST_ENABLED; + HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock); return 1; } -/* parse a "disable server" command. It always returns 1. */ +/* parse a "disable server" command. It always returns 1. + * + * Grabs the server lock. + */ static int cli_parse_disable_server(char **args, struct appctx *appctx, void *private) { struct server *sv; @@ -4386,11 +4416,16 @@ static int cli_parse_disable_server(char **args, struct appctx *appctx, void *pr if (!sv) return 1; + HA_SPIN_LOCK(SERVER_LOCK, &sv->lock); srv_adm_set_maint(sv); + HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock); return 1; } -/* parse a "enable agent" command. It always returns 1. */ +/* parse a "enable agent" command. It always returns 1. + * + * Grabs the server lock. + */ static int cli_parse_enable_agent(char **args, struct appctx *appctx, void *private) { struct server *sv; @@ -4409,11 +4444,16 @@ static int cli_parse_enable_agent(char **args, struct appctx *appctx, void *priv return 1; } + HA_SPIN_LOCK(SERVER_LOCK, &sv->lock); sv->agent.state |= CHK_ST_ENABLED; + HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock); return 1; } -/* parse a "enable health" command. It always returns 1. */ +/* parse a "enable health" command. It always returns 1. + * + * Grabs the server lock. + */ static int cli_parse_enable_health(char **args, struct appctx *appctx, void *private) { struct server *sv; @@ -4425,11 +4465,16 @@ static int cli_parse_enable_health(char **args, struct appctx *appctx, void *pri if (!sv) return 1; + HA_SPIN_LOCK(SERVER_LOCK, &sv->lock); sv->check.state |= CHK_ST_ENABLED; + HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock); return 1; } -/* parse a "enable server" command. It always returns 1. */ +/* parse a "enable server" command. It always returns 1. + * + * Grabs the server lock. + */ static int cli_parse_enable_server(char **args, struct appctx *appctx, void *private) { struct server *sv; @@ -4441,11 +4486,13 @@ static int cli_parse_enable_server(char **args, struct appctx *appctx, void *pri if (!sv) return 1; + HA_SPIN_LOCK(SERVER_LOCK, &sv->lock); srv_adm_set_ready(sv); if (!(sv->flags & SRV_F_COOKIESET) && (sv->proxy->ck_opts & PR_CK_DYNAMIC) && sv->cookie) srv_check_for_dup_dyncookie(sv); + HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock); return 1; }