File 0004-CLEANUP-checks-fix-double-usage-of-cur-cur... of Package haproxy

Overview Repositories Revisions Requests Users Attributes Meta

File 0004-CLEANUP-checks-fix-double-usage-of-cur-current_step-.patch of Package haproxy (Revision 4c15d160769a9597091c7c6236cceb3d)

Currently displaying revision 4c15d160769a9597091c7c6236cceb3d , Show latest

From 4f889006269e4d3f802de46f280ed198a15e3a69 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Wed, 13 May 2015 11:23:01 +0200
Subject: [PATCH 4/9] CLEANUP: checks: fix double usage of cur / current_step
 in tcp-checks

This cleanup is a preliminary requirement to the upcoming fixes for
the bug that affect tcp-check's improper use of lists. It will have
to be backported to 1.5 though it will not easily apply.

There are two variables pointing to the current rule within the loop,
and either one or the other is used depending on the code blocks,
making it much harder to apply checks to fix the list walking bug.
So first get rid of "cur" and only focus on current_step.
(cherry picked from commit ce8c42a37a44a1e0cb94e81abb7cc2baf3d0ef80)

[wt: 1.5 doesn't have comments so this patch differs significantly
 from 1.6, but it's needed for the next batch of fixes]
---
 src/checks.c | 57 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/src/checks.c b/src/checks.c
index 8b53f97..cfdfe8c 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -1859,7 +1859,7 @@ static int tcpcheck_get_step_id(struct server *s)
 static void tcpcheck_main(struct connection *conn)
 {
 	char *contentptr;
-	struct tcpcheck_rule *cur, *next;
+	struct tcpcheck_rule *next;
 	int done = 0, ret = 0;
 	struct check *check = conn->owner;
 	struct server *s = check->server;
@@ -1916,15 +1916,11 @@ static void tcpcheck_main(struct connection *conn)
 		check->bo->o = 0;
 		check->bi->p = check->bi->data;
 		check->bi->i = 0;
-		cur = check->current_step = LIST_ELEM(head->n, struct tcpcheck_rule *, list);
+		check->current_step = LIST_ELEM(head->n, struct tcpcheck_rule *, list);
 		t->expire = tick_add(now_ms, MS_TO_TICKS(check->inter));
 		if (s->proxy->timeout.check)
 			t->expire = tick_add_ifset(now_ms, s->proxy->timeout.check);
 	}
-	/* keep on processing step */
-	else {
-		cur = check->current_step;
-	}
 
 	/* It's only the rules which will enable send/recv */
 	__conn_data_stop_both(conn);
@@ -1934,7 +1930,7 @@ static void tcpcheck_main(struct connection *conn)
 		 * or if we're about to send a string that does not fit in the remaining space.
 		 */
 		if (check->bo->o &&
-		    (&cur->list == head ||
+		    (&check->current_step->list == head ||
 		     check->current_step->action != TCPCHK_ACT_SEND ||
 		     check->current_step->string_len >= buffer_total_space(check->bo))) {
 
@@ -1949,14 +1945,17 @@ static void tcpcheck_main(struct connection *conn)
 		}
 
 		/* did we reach the end ? If so, let's check that everything was sent */
-		if (&cur->list == head) {
+		if (&check->current_step->list == head) {
 			if (check->bo->o)
 				goto out_need_io;
 			break;
 		}
 
-		/* have 'next' point to the next rule or NULL if we're on the last one */
-		next = (struct tcpcheck_rule *)cur->list.n;
+		/* 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;
+
 		if (&next->list == head)
 			next = NULL;
 
@@ -2058,8 +2057,7 @@ static void tcpcheck_main(struct connection *conn)
 			}
 
 			/* allow next rule */
-			cur = (struct tcpcheck_rule *)cur->list.n;
-			check->current_step = cur;
+			check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
 
 			/* don't do anything until the connection is established */
 			if (!(conn->flags & CO_FL_CONNECTED)) {
@@ -2113,8 +2111,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 */
-			cur = (struct tcpcheck_rule *)cur->list.n;
-			check->current_step = cur;
+			check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
 		} /* end 'send' */
 		else if (check->current_step->action == TCPCHK_ACT_EXPECT) {
 			if (unlikely(check->result == CHK_RES_FAILED))
@@ -2167,14 +2164,14 @@ static void tcpcheck_main(struct connection *conn)
 				goto out_end_tcpcheck;
 			}
 
-			if (!done && (cur->string != NULL) && (check->bi->i < cur->string_len) )
+			if (!done && (check->current_step->string != NULL) && (check->bi->i < check->current_step->string_len) )
 				continue; /* try to read more */
 
 		tcpcheck_expect:
-			if (cur->string != NULL)
-				ret = my_memmem(contentptr, check->bi->i, cur->string, cur->string_len) != NULL;
-			else if (cur->expect_regex != NULL)
-				ret = regex_exec(cur->expect_regex, contentptr);
+			if (check->current_step->string != NULL)
+				ret = my_memmem(contentptr, check->bi->i, check->current_step->string, check->current_step->string_len) != NULL;
+			else if (check->current_step->expect_regex != NULL)
+				ret = regex_exec(check->current_step->expect_regex, contentptr);
 
 			if (!ret && !done)
 				continue; /* try to read more */
@@ -2182,11 +2179,11 @@ static void tcpcheck_main(struct connection *conn)
 			/* matched */
 			if (ret) {
 				/* matched but we did not want to => ERROR */
-				if (cur->inverse) {
+				if (check->current_step->inverse) {
 					/* we were looking for a string */
-					if (cur->string != NULL) {
+					if (check->current_step->string != NULL) {
 						chunk_printf(&trash, "TCPCHK matched unwanted content '%s' at step %d",
-								cur->string, tcpcheck_get_step_id(s));
+						             check->current_step->string, tcpcheck_get_step_id(s));
 					}
 					else {
 					/* we were looking for a regex */
@@ -2198,8 +2195,9 @@ static void tcpcheck_main(struct connection *conn)
 				}
 				/* matched and was supposed to => OK, next step */
 				else {
-					cur = (struct tcpcheck_rule*)cur->list.n;
-					check->current_step = cur;
+					/* allow next rule */
+					check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+
 					if (check->current_step->action == TCPCHK_ACT_EXPECT)
 						goto tcpcheck_expect;
 					__conn_data_stop_recv(conn);
@@ -2208,9 +2206,10 @@ static void tcpcheck_main(struct connection *conn)
 			else {
 			/* not matched */
 				/* not matched and was not supposed to => OK, next step */
-				if (cur->inverse) {
-					cur = (struct tcpcheck_rule*)cur->list.n;
-					check->current_step = cur;
+				if (check->current_step->inverse) {
+					/* allow next rule */
+					check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+
 					if (check->current_step->action == TCPCHK_ACT_EXPECT)
 						goto tcpcheck_expect;
 					__conn_data_stop_recv(conn);
@@ -2218,9 +2217,9 @@ static void tcpcheck_main(struct connection *conn)
 				/* not matched but was supposed to => ERROR */
 				else {
 					/* we were looking for a string */
-					if (cur->string != NULL) {
+					if (check->current_step->string != NULL) {
 						chunk_printf(&trash, "TCPCHK did not match content '%s' at step %d",
-								cur->string, tcpcheck_get_step_id(s));
+						             check->current_step->string, tcpcheck_get_step_id(s));
 					}
 					else {
 					/* we were looking for a regex */
-- 
2.3.7

Places

File 0004-CLEANUP-checks-fix-double-usage-of-cur-current_step-.patch of Package haproxy (Revision 4c15d160769a9597091c7c6236cceb3d)

Places