File 0008-BUG-MEDIUM-checks-do-not-dereference-a-lis... of Package haproxy

Overview Repositories Revisions Requests Users Attributes Meta

File 0008-BUG-MEDIUM-checks-do-not-dereference-a-list-as-a-tcp.patch of Package haproxy (Revision 4c15d160769a9597091c7c6236cceb3d)

Currently displaying revision 4c15d160769a9597091c7c6236cceb3d , Show latest

From 5bff05986c501d9ffb67873b60472f9c2a2e41be Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Wed, 13 May 2015 12:24:53 +0200
Subject: [PATCH 8/9] BUG/MEDIUM: checks: do not dereference a list as a
 tcpcheck struct

The method used to skip to next rule in the list is wrong, it assumes
that the list element starts at the same offset as the rule. It happens
to be true on most architectures since the list is the first element for
now but it's definitely wrong. Now the code doesn't crash anymore when
the struct list is moved anywhere else in the struct tcpcheck_rule.

This fix must be backported to 1.5.
(cherry picked from commit 5581c27b579cbfc53afb0ca04cdeebe7e2200131)
[wt: changes from 1.6 : no tcp-check comments, check becomes s->proxy]
---
 src/cfgparse.c | 18 +++++++-----------
 src/checks.c   | 15 +++++++++------
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index dba59d1..e04eff8 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -4362,20 +4362,16 @@ stats_error_parsing:
 			const char *ptr_arg;
 			int cur_arg;
 			struct tcpcheck_rule *tcpcheck;
-			struct list *l;
 
 			/* check if first rule is also a 'connect' action */
-			l = (struct list *)&curproxy->tcpcheck_rules;
-			if (l->p != l->n) {
-				tcpcheck = (struct tcpcheck_rule *)l->n;
+			tcpcheck = LIST_NEXT(&curproxy->tcpcheck_rules, struct tcpcheck_rule *, list);
 
-				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;
-					goto out;
-				}
+			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;
+				goto out;
 			}
 
 			cur_arg = 2;
diff --git a/src/checks.c b/src/checks.c
index e13d561..27a23b2 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -1444,7 +1444,10 @@ static int connect_chk(struct task *t)
 	quickack = check->type == 0 || check->type == PR_O2_TCPCHK_CHK;
 
 	if (check->type == PR_O2_TCPCHK_CHK && !LIST_ISEMPTY(&s->proxy->tcpcheck_rules)) {
-		struct tcpcheck_rule *r = (struct tcpcheck_rule *) s->proxy->tcpcheck_rules.n;
+		struct tcpcheck_rule *r;
+
+		r = LIST_NEXT(&s->proxy->tcpcheck_rules, struct tcpcheck_rule *, list);
+
 		/* if first step is a 'connect', then tcpcheck_main must run it */
 		if (r->action == TCPCHK_ACT_CONNECT) {
 			tcpcheck_main(conn);
@@ -1952,7 +1955,7 @@ static void tcpcheck_main(struct connection *conn)
 		/* have 'next' point to the next rule or NULL if we're on the
 		 * last one, connect() needs this.
 		 */
-		next = (struct tcpcheck_rule *)check->current_step->list.n;
+		next = LIST_NEXT(&check->current_step->list, struct tcpcheck_rule *, list);
 
 		if (&next->list == head)
 			next = NULL;
@@ -2055,7 +2058,7 @@ static void tcpcheck_main(struct connection *conn)
 			}
 
 			/* allow next rule */
-			check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+			check->current_step = LIST_NEXT(&check->current_step->list, struct tcpcheck_rule *, list);
 
 			if (&check->current_step->list == head)
 				break;
@@ -2112,7 +2115,7 @@ static void tcpcheck_main(struct connection *conn)
 			*check->bo->p = '\0'; /* to make gdb output easier to read */
 
 			/* go to next rule and try to send */
-			check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+			check->current_step = LIST_NEXT(&check->current_step->list, struct tcpcheck_rule *, list);
 
 			if (&check->current_step->list == head)
 				break;
@@ -2200,7 +2203,7 @@ static void tcpcheck_main(struct connection *conn)
 				/* matched and was supposed to => OK, next step */
 				else {
 					/* allow next rule */
-					check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+					check->current_step = LIST_NEXT(&check->current_step->list, struct tcpcheck_rule *, list);
 
 					if (&check->current_step->list == head)
 						break;
@@ -2215,7 +2218,7 @@ static void tcpcheck_main(struct connection *conn)
 				/* not matched and was not supposed to => OK, next step */
 				if (check->current_step->inverse) {
 					/* allow next rule */
-					check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+					check->current_step = LIST_NEXT(&check->current_step->list, struct tcpcheck_rule *, list);
 
 					if (&check->current_step->list == head)
 						break;
-- 
2.3.7

Places

File 0008-BUG-MEDIUM-checks-do-not-dereference-a-list-as-a-tcp.patch of Package haproxy (Revision 4c15d160769a9597091c7c6236cceb3d)

Places