BUG/MEDIUM: proxy: ensure pause_proxy() and resume_proxy() own PROXY_LOCK
There was a race involving hlua_proxy_* functions
and some proxy management functions.
pause_proxy() and resume_proxy() can be used directly from lua code,
but that could lead to some race as lua code didn't make sure PROXY_LOCK
was owned before calling the proxy functions.
This patch makes sure it won't happen again elsewhere in the code
by locking PROXY_LOCK directly in resume and pause proxy functions
so that it's not the caller's responsibility anymore.
(based on stop_proxy() behavior that was already safe prior to the patch)
This should be backported to stable series.
Note that the API will likely differ < 2.4
diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
index 5907d48..0046e9e 100644
--- a/src/hlua_fcn.c
+++ b/src/hlua_fcn.c
@@ -1311,6 +1311,7 @@
struct proxy *px;
px = hlua_check_proxy(L, 1);
+ /* safe to call without PROXY_LOCK - pause_proxy takes it */
pause_proxy(px);
return 0;
}
@@ -1320,6 +1321,7 @@
struct proxy *px;
px = hlua_check_proxy(L, 1);
+ /* safe to call without PROXY_LOCK - resume_proxy takes it */
resume_proxy(px);
return 0;
}
@@ -1329,6 +1331,7 @@
struct proxy *px;
px = hlua_check_proxy(L, 1);
+ /* safe to call without PROXY_LOCK - stop_proxy takes it */
stop_proxy(px);
return 0;
}
diff --git a/src/proxy.c b/src/proxy.c
index 6fad8ee..7b1c96e 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -2252,13 +2252,17 @@
/* Temporarily disables listening on all of the proxy's listeners. Upon
* success, the proxy enters the PR_PAUSED state. The function returns 0
* if it fails, or non-zero on success.
+ * The function takes the proxy's lock so it's safe to
+ * call from multiple places.
*/
int pause_proxy(struct proxy *p)
{
struct listener *l;
+ HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
+
if (!(p->cap & PR_CAP_FE) || (p->flags & (PR_FL_DISABLED|PR_FL_STOPPED)) || !p->li_ready)
- return 1;
+ goto end;
list_for_each_entry(l, &p->conf.listeners, by_fe)
pause_listener(l);
@@ -2266,8 +2270,11 @@
if (p->li_ready) {
ha_warning("%s %s failed to enter pause mode.\n", proxy_cap_str(p->cap), p->id);
send_log(p, LOG_WARNING, "%s %s failed to enter pause mode.\n", proxy_cap_str(p->cap), p->id);
+ HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &p->lock);
return 0;
}
+end:
+ HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &p->lock);
return 1;
}
@@ -2276,7 +2283,8 @@
* to be called when going down in order to release the ports so that another
* process may bind to them. It must also be called on disabled proxies at the
* end of start-up. If all listeners are closed, the proxy is set to the
- * PR_STOPPED state. The function takes the proxy's lock so it's safe to
+ * PR_STOPPED state.
+ * The function takes the proxy's lock so it's safe to
* call from multiple places.
*/
void stop_proxy(struct proxy *p)
@@ -2300,14 +2308,18 @@
* listeners and tries to enable them all. If any of them fails, the proxy is
* put back to the paused state. It returns 1 upon success, or zero if an error
* is encountered.
+ * The function takes the proxy's lock so it's safe to
+ * call from multiple places.
*/
int resume_proxy(struct proxy *p)
{
struct listener *l;
int fail;
+ HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
+
if ((p->flags & (PR_FL_DISABLED|PR_FL_STOPPED)) || !p->li_paused)
- return 1;
+ goto end;
fail = 0;
list_for_each_entry(l, &p->conf.listeners, by_fe) {
@@ -2335,9 +2347,13 @@
}
if (fail) {
+ HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &p->lock);
+ /* pause_proxy will take PROXY_LOCK */
pause_proxy(p);
return 0;
}
+end:
+ HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &p->lock);
return 1;
}
@@ -3051,9 +3067,8 @@
if (!px->li_ready)
return cli_msg(appctx, LOG_NOTICE, "All sockets are already disabled.\n");
- HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
+ /* pause_proxy will take PROXY_LOCK */
ret = pause_proxy(px);
- HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock);
if (!ret)
return cli_err(appctx, "Failed to pause frontend, check logs for precise cause.\n");
@@ -3083,9 +3098,8 @@
if (px->li_ready == px->li_all)
return cli_msg(appctx, LOG_NOTICE, "All sockets are already enabled.\n");
- HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
+ /* resume_proxy will take PROXY_LOCK */
ret = resume_proxy(px);
- HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock);
if (!ret)
return cli_err(appctx, "Failed to resume frontend, check logs for precise cause (port conflict?).\n");