From 3bcc2699ba08dd3971ae7a56631994b2524d2acb Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 21 Aug 2018 15:35:31 +0200 Subject: [PATCH] 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. --- src/server.c | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/src/server.c b/src/server.c index 871a28dc3..b73cc2eea 100644 --- a/src/server.c +++ b/src/server.c @@ -4420,8 +4420,9 @@ static int cli_parse_get_weight(char **args, char *payload, struct appctx *appct return 1; } -/* - * Must be called with the server lock held. +/* Parse a "set weight" command. + * + * Grabs the server lock. */ static int cli_parse_set_weight(char **args, char *payload, struct appctx *appctx, void *private) { @@ -4435,18 +4436,23 @@ static int cli_parse_set_weight(char **args, char *payload, struct appctx *appct 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. * - * Must be called with the server lock held. + * Grabs the server lock. */ static int cli_parse_set_maxconn_server(char **args, char *payload, struct appctx *appctx, void *private) { @@ -4460,18 +4466,23 @@ static int cli_parse_set_maxconn_server(char **args, char *payload, struct appct 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. * - * Must be called with the server lock held. + * Grabs the server lock. */ static int cli_parse_disable_agent(char **args, char *payload, struct appctx *appctx, void *private) { @@ -4484,13 +4495,15 @@ static int cli_parse_disable_agent(char **args, char *payload, struct appctx *ap 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. * - * Must be called with the server lock held. + * Grabs the server lock. */ static int cli_parse_disable_health(char **args, char *payload, struct appctx *appctx, void *private) { @@ -4503,13 +4516,15 @@ static int cli_parse_disable_health(char **args, char *payload, struct appctx *a 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. * - * Must be called with the server lock held. + * Grabs the server lock. */ static int cli_parse_disable_server(char **args, char *payload, struct appctx *appctx, void *private) { @@ -4522,13 +4537,15 @@ static int cli_parse_disable_server(char **args, char *payload, struct appctx *a 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. * - * Must be called with the server lock held. + * Grabs the server lock. */ static int cli_parse_enable_agent(char **args, char *payload, struct appctx *appctx, void *private) { @@ -4548,13 +4565,15 @@ static int cli_parse_enable_agent(char **args, char *payload, struct appctx *app 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. * - * Must be called with the server lock held. + * Grabs the server lock. */ static int cli_parse_enable_health(char **args, char *payload, struct appctx *appctx, void *private) { @@ -4567,13 +4586,15 @@ static int cli_parse_enable_health(char **args, char *payload, struct appctx *ap 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. * - * Must be called with the server lock held. + * Grabs the server lock. */ static int cli_parse_enable_server(char **args, char *payload, struct appctx *appctx, void *private) { @@ -4586,11 +4607,13 @@ static int cli_parse_enable_server(char **args, char *payload, struct appctx *ap 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; } -- 2.47.3