konevsa/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

BUG/MEDIUM: proxy: ensure pause_proxy() and resume_proxy() own PROXY_LOCK

Browse Source 
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
This commit is contained in:
Aurelien DARRAGON 2022-09-08 14:35:35 +02:00 committed by Christopher Faulet
parent 6edae6ff48
commit 7d00077fd5
2 changed files with 24 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/hlua_fcn.c
View File

@ -1311,6 +1311,7 @@ int hlua_proxy_pause(lua_State *L)
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 @@ int hlua_proxy_resume(lua_State *L)
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 @@ int hlua_proxy_stop(lua_State *L)
struct proxy *px;
px = hlua_check_proxy(L, 1);
/* safe to call without PROXY_LOCK - stop_proxy takes it */
stop_proxy(px);
return 0;
}

28
src/proxy.c
View File

@ -2252,13 +2252,17 @@ void soft_stop(void)
/* 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 @@ int pause_proxy(struct proxy *p)
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 @@ int pause_proxy(struct proxy *p)
* 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 @@ void stop_proxy(struct proxy *p)
* 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 @@ int resume_proxy(struct proxy *p)
}
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 @@ static int cli_parse_disable_frontend(char **args, char *payload, struct appctx
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 @@ static int cli_parse_enable_frontend(char **args, char *payload, struct appctx *
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");