mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-11-24 12:20:59 +01:00
Code Issues Projects Releases Wiki Activity

BUG/MAJOR: checks: always check for end of list before proceeding

Browse Source 
This is the most important fix of this series. There's a risk of endless
loop and crashes caused by the fact that we go past the head of the list
when skipping to next rule, without checking if it's still a valid element.
Most of the time, the ->action field is checked, which points to the proxy's
check_req pointer (generally NULL), meaning the element is confused with a
TCPCHK_ACT_SEND action.

The situation was accidently made worse with the addition of tcp-check
comment since it also skips list elements. However, since the action that
makes it go forward is TCPCHK_ACT_COMMENT (3), there's little chance to
see this as a valid pointer, except on 64-bit machines where it can match
the end of a check_req string pointer.

This fix heavily depends on previous cleanup and both must be backported
to 1.5 where the bug is present.
This commit is contained in:
Willy Tarreau 2015-05-13 12:08:21 +02:00
parent 263013d031
commit f2c87353a7
2 changed files with 25 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/cfgparse.c
View File

@ -4712,14 +4712,13 @@ stats_error_parsing:
l = (struct list *)&curproxy->tcpcheck_rules;
if (l->p != l->n) {
tcpcheck = (struct tcpcheck_rule *)l->n;
while (tcpcheck->action == TCPCHK_ACT_COMMENT) {
while (&tcpcheck->list != &curproxy->tcpcheck_rules &&
tcpcheck->action == TCPCHK_ACT_COMMENT) {
tcpcheck = (struct tcpcheck_rule *)tcpcheck->list.n;
}
/* we've reached the end of the list, and the list is full of comments */
if (tcpcheck == (struct tcpcheck_rule *)l)
tcpcheck = NULL;
if (tcpcheck && tcpcheck->action != TCPCHK_ACT_CONNECT) {
if (&tcpcheck->list != &curproxy->tcpcheck_rules
&& tcpcheck->action != TCPCHK_ACT_CONNECT) {
Alert("parsing [%s:%d] : first step MUST also be a 'connect' when there is a 'connect' step in the tcp-check ruleset.\n",
file, linenum);
err_code |= ERR_ALERT | ERR_FATAL;

26
src/checks.c
View File

@ -2522,7 +2522,7 @@ static void tcpcheck_main(struct connection *conn)
next = (struct tcpcheck_rule *)check->current_step->list.n;
/* bypass all comment rules */
while (next->action == TCPCHK_ACT_COMMENT)
while (&next->list != head && next->action == TCPCHK_ACT_COMMENT)
next = (struct tcpcheck_rule *)next->list.n;
/* NULL if we're on the last rule */
@ -2636,9 +2636,13 @@ static void tcpcheck_main(struct connection *conn)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
/* bypass all comment rules */
while (check->current_step->action == TCPCHK_ACT_COMMENT)
while (&check->current_step->list != head &&
check->current_step->action == TCPCHK_ACT_COMMENT)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
if (&check->current_step->list == head)
break;
/* don't do anything until the connection is established */
if (!(conn->flags & CO_FL_CONNECTED)) {
/* update expire time, should be done by process_chk */
@ -2694,8 +2698,12 @@ static void tcpcheck_main(struct connection *conn)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
/* bypass all comment rules */
while (check->current_step->action == TCPCHK_ACT_COMMENT)
while (&check->current_step->list != head &&
check->current_step->action == TCPCHK_ACT_COMMENT)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
if (&check->current_step->list == head)
break;
} /* end 'send' */
else if (check->current_step->action == TCPCHK_ACT_EXPECT) {
if (unlikely(check->result == CHK_RES_FAILED))
@ -2789,9 +2797,13 @@ static void tcpcheck_main(struct connection *conn)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
/* bypass all comment rules */
while (check->current_step->action == TCPCHK_ACT_COMMENT)
while (&check->current_step->list != head &&
check->current_step->action == TCPCHK_ACT_COMMENT)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
if (&check->current_step->list == head)
break;
if (check->current_step->action == TCPCHK_ACT_EXPECT)
goto tcpcheck_expect;
__conn_data_stop_recv(conn);
@ -2805,9 +2817,13 @@ static void tcpcheck_main(struct connection *conn)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
/* bypass all comment rules */
while (check->current_step->action == TCPCHK_ACT_COMMENT)
while (&check->current_step->list != head &&
check->current_step->action == TCPCHK_ACT_COMMENT)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
if (&check->current_step->list == head)
break;
if (check->current_step->action == TCPCHK_ACT_EXPECT)
goto tcpcheck_expect;
__conn_data_stop_recv(conn);