RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2024-12-26 06:32:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

BUG/MAJOR: cli: explicitly call cli_release_handler() upon error

Browse Source 
Dmitry Sivachenko reported an embarrassing problem where haproxy
would sometimes segfault upon reload. After careful analysis and
code inspection, what happens is related to the "show sess" command
on the CLI, and it is not limited to reload operations only.

When a "show sess" is running, once the output buffer is full, the
stats applet grabs a reference to the session being dumped in order
for the current pointer to be able to advance by itself should this
session disappear while the buffer is full. The applet also uses a
release handler that is called when the applet terminates to release
such references.

The problem is that upon error, the command line parser sets the
applet state to STAT_CLI_O_END indicating it wants to terminate the
processing. Unfortunately, the release handler which is called later
to clean everything up relies on the applet's state to know what
operations were in progress, and as such it does not release the
reference. A later "show sess" or the completion of the task being
watched lead to a LIST_DEL() on the task's list which point to a
location that does not match the applet's reference list anymore
and the process dies.

One solution to this would be to add a flag to the current applet's
state mentionning it must leave, without affecting the state indicating
the current operation. It's a bit invasive but could be the long term
solution. The short term solution simply consists in calling the
release handler just before changing the state to STAT_CLI_O_END.
That way everything that must be released is released in time.

Note that the probability to encounter this issue is very low.
It requires a lot of "show sess" or "show sess all" calls, and
that one of them dies before being completed. That can happen
if "show sess" is run in scripts which truncate the output (eg:
"echo show sess|socat|head"). This could be the worst case as it
almost ensures that haproxy fills a buffer, grabs a reference and
detects the error on the socket.

There's no config-based workaround to this issue, except refraining
from issuing "show sess" on large connection counts or "show sess all".
If that's not possible to block everyone, restricting permissions on
the stats socket ensures only authorized tools can connect.

This fix must be backported to 1.5 and to 1.4 (with some changes in
1.4 since the release function does not exist so the LIST_DEL sequence
must be open-coded).

Special thanks to Dmitry for the fairly complete report.
This commit is contained in:
Willy Tarreau 2014-10-22 19:06:31 +02:00
parent 7d59e90473
commit 44cf545000
1 changed files with 3 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/dumpstats.c
View File

@ -131,6 +131,7 @@ static int stats_dump_stat_to_buffer(struct stream_interface *si, struct uri_aut
static int stats_pats_list(struct stream_interface *si);
static int stats_pat_list(struct stream_interface *si);
static int stats_map_lookup(struct stream_interface *si);
static void cli_release_handler(struct stream_interface *si);
/*
* cli_io_handler()
@ -2336,6 +2337,7 @@ static void cli_io_handler(struct stream_interface *si)
}
else { /* output functions: first check if the output buffer is closed then abort */
if (res->flags & (CF_SHUTR_NOW|CF_SHUTR)) {
cli_release_handler(si);
appctx->st0 = STAT_CLI_END;
continue;
}
@ -2389,6 +2391,7 @@ static void cli_io_handler(struct stream_interface *si)
appctx->st0 = STAT_CLI_PROMPT;
break;
default: /* abnormal state */
cli_release_handler(si);
appctx->st0 = STAT_CLI_PROMPT;
break;
}