BUG/MEDIUM: cli/threads: protect all "proxy" commands against concurrent updates
The proxy-related commands like "{enable|disable|shutdown} frontend",
"{enable|disable} dynamic-cookie", "set dynamic-cookie-key" were not
protected against concurrent accesses making their use dangerous with
threads.
This patch must be backported to 1.8.
diff --git a/src/proxy.c b/src/proxy.c
index ff094d2..73aa118 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -1560,7 +1560,10 @@
return 1;
}
-/* Parses the "enable dynamic-cookies backend" directive, it always returns 1 */
+/* Parses the "enable dynamic-cookies backend" directive, it always returns 1.
+ *
+ * Grabs the proxy lock and each server's lock.
+ */
static int cli_parse_enable_dyncookie_backend(char **args, char *payload, struct appctx *appctx, void *private)
{
struct proxy *px;
@@ -1573,15 +1576,25 @@
if (!px)
return 1;
+ HA_SPIN_LOCK(PROXY_LOCK, &px->lock);
+
px->ck_opts |= PR_CK_DYNAMIC;
- for (s = px->srv; s != NULL; s = s->next)
+ for (s = px->srv; s != NULL; s = s->next) {
+ HA_SPIN_LOCK(SERVER_LOCK, &s->lock);
srv_set_dyncookie(s);
+ HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);
+ }
+
+ HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock);
return 1;
}
-/* Parses the "disable dynamic-cookies backend" directive, it always returns 1 */
+/* Parses the "disable dynamic-cookies backend" directive, it always returns 1.
+ *
+ * Grabs the proxy lock and each server's lock.
+ */
static int cli_parse_disable_dyncookie_backend(char **args, char *payload, struct appctx *appctx, void *private)
{
struct proxy *px;
@@ -1594,19 +1607,28 @@
if (!px)
return 1;
+ HA_SPIN_LOCK(PROXY_LOCK, &px->lock);
+
px->ck_opts &= ~PR_CK_DYNAMIC;
for (s = px->srv; s != NULL; s = s->next) {
+ HA_SPIN_LOCK(SERVER_LOCK, &s->lock);
if (!(s->flags & SRV_F_COOKIESET)) {
free(s->cookie);
s->cookie = NULL;
}
+ HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);
}
+ HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock);
+
return 1;
}
-/* Parses the "set dynamic-cookie-key backend" directive, it always returns 1 */
+/* Parses the "set dynamic-cookie-key backend" directive, it always returns 1.
+ *
+ * Grabs the proxy lock and each server's lock.
+ */
static int cli_parse_set_dyncookie_key_backend(char **args, char *payload, struct appctx *appctx, void *private)
{
struct proxy *px;
@@ -1634,16 +1656,27 @@
appctx->st0 = CLI_ST_PRINT;
return 1;
}
+
+ HA_SPIN_LOCK(PROXY_LOCK, &px->lock);
+
free(px->dyncookie_key);
px->dyncookie_key = newkey;
- for (s = px->srv; s != NULL; s = s->next)
+ for (s = px->srv; s != NULL; s = s->next) {
+ HA_SPIN_LOCK(SERVER_LOCK, &s->lock);
srv_set_dyncookie(s);
+ HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);
+ }
+
+ HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock);
return 1;
}
-/* Parses the "set maxconn frontend" directive, it always returns 1 */
+/* Parses the "set maxconn frontend" directive, it always returns 1.
+ *
+ * Grabs the proxy lock.
+ */
static int cli_parse_set_maxconn_frontend(char **args, char *payload, struct appctx *appctx, void *private)
{
struct proxy *px;
@@ -1675,6 +1708,8 @@
/* OK, the value is fine, so we assign it to the proxy and to all of
* its listeners. The blocked ones will be dequeued.
*/
+ HA_SPIN_LOCK(PROXY_LOCK, &px->lock);
+
px->maxconn = v;
list_for_each_entry(l, &px->conf.listeners, by_fe) {
l->maxconn = v;
@@ -1685,10 +1720,15 @@
if (px->maxconn > px->feconn && !LIST_ISEMPTY(&px->listener_queue))
dequeue_all_listeners(&px->listener_queue);
+ HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock);
+
return 1;
}
-/* Parses the "shutdown frontend" directive, it always returns 1 */
+/* Parses the "shutdown frontend" directive, it always returns 1.
+ *
+ * Grabs the proxy lock.
+ */
static int cli_parse_shutdown_frontend(char **args, char *payload, struct appctx *appctx, void *private)
{
struct proxy *px;
@@ -1711,14 +1751,22 @@
px->id, px->fe_counters.cum_conn, px->be_counters.cum_conn);
send_log(px, LOG_WARNING, "Proxy %s stopped (FE: %lld conns, BE: %lld conns).\n",
px->id, px->fe_counters.cum_conn, px->be_counters.cum_conn);
+
+ HA_SPIN_LOCK(PROXY_LOCK, &px->lock);
stop_proxy(px);
+ HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock);
+
return 1;
}
-/* Parses the "disable frontend" directive, it always returns 1 */
+/* Parses the "disable frontend" directive, it always returns 1.
+ *
+ * Grabs the proxy lock.
+ */
static int cli_parse_disable_frontend(char **args, char *payload, struct appctx *appctx, void *private)
{
struct proxy *px;
+ int ret;
if (!cli_has_level(appctx, ACCESS_LVL_ADMIN))
return 1;
@@ -1741,7 +1789,11 @@
return 1;
}
- if (!pause_proxy(px)) {
+ HA_SPIN_LOCK(PROXY_LOCK, &px->lock);
+ ret = pause_proxy(px);
+ HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock);
+
+ if (!ret) {
appctx->ctx.cli.severity = LOG_ERR;
appctx->ctx.cli.msg = "Failed to pause frontend, check logs for precise cause.\n";
appctx->st0 = CLI_ST_PRINT;
@@ -1750,10 +1802,14 @@
return 1;
}
-/* Parses the "enable frontend" directive, it always returns 1 */
+/* Parses the "enable frontend" directive, it always returns 1.
+ *
+ * Grabs the proxy lock.
+ */
static int cli_parse_enable_frontend(char **args, char *payload, struct appctx *appctx, void *private)
{
struct proxy *px;
+ int ret;
if (!cli_has_level(appctx, ACCESS_LVL_ADMIN))
return 1;
@@ -1776,7 +1832,11 @@
return 1;
}
- if (!resume_proxy(px)) {
+ HA_SPIN_LOCK(PROXY_LOCK, &px->lock);
+ ret = resume_proxy(px);
+ HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock);
+
+ if (!ret) {
appctx->ctx.cli.severity = LOG_ERR;
appctx->ctx.cli.msg = "Failed to resume frontend, check logs for precise cause (port conflict?).\n";
appctx->st0 = CLI_ST_PRINT;