File 0007-BUG-MAJOR-checks-always-check-for-end-of-l... of Package haproxy

Overview Repositories Revisions Requests Users Attributes Meta

File 0007-BUG-MAJOR-checks-always-check-for-end-of-list-before.patch of Package haproxy (Revision 4c15d160769a9597091c7c6236cceb3d)

Currently displaying revision 4c15d160769a9597091c7c6236cceb3d , Show latest

From 97fccc87f1297d189ee80735e5b8746c34956eda Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Wed, 13 May 2015 12:08:21 +0200
Subject: [PATCH 7/9] BUG/MAJOR: checks: always check for end of list before
 proceeding

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.
(cherry picked from commit f2c87353a7f8160930b5f342bb6d6ad0991ee3d1)
[wt: this patch differs significantly from 1.6 since we don't have comments]
---
 src/cfgparse.c |  4 +++-
 src/checks.c   | 12 ++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 746c7eb..dba59d1 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -4368,7 +4368,9 @@ stats_error_parsing:
 			l = (struct list *)&curproxy->tcpcheck_rules;
 			if (l->p != l->n) {
 				tcpcheck = (struct tcpcheck_rule *)l->n;
-				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;
diff --git a/src/checks.c b/src/checks.c
index a0c42f2..e13d561 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -2057,6 +2057,9 @@ static void tcpcheck_main(struct connection *conn)
 			/* allow next rule */
 			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 */
@@ -2110,6 +2113,9 @@ static void tcpcheck_main(struct connection *conn)
 
 			/* go to next rule and try to send */
 			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))
@@ -2196,6 +2202,9 @@ static void tcpcheck_main(struct connection *conn)
 					/* allow next rule */
 					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);
@@ -2208,6 +2217,9 @@ static void tcpcheck_main(struct connection *conn)
 					/* allow next rule */
 					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);
-- 
2.3.7

Places

File 0007-BUG-MAJOR-checks-always-check-for-end-of-list-before.patch of Package haproxy (Revision 4c15d160769a9597091c7c6236cceb3d)

Places