From: Christopher Faulet <cfaulet@haproxy.com>
Date: Thu, 7 Nov 2019 13:27:52 +0000 (+0100)
Subject: BUG/MEDIUM: stream: Be sure to release allocated captures for TCP streams
X-Git-Tag: v2.1-dev5~32
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5939925a3805f9755cff5a3b9635c1a533bc9184;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: stream: Be sure to release allocated captures for TCP streams

All TCP and HTTP captures are stored in 2 arrays, one for the request and
another for the response. In HAPRoxy 1.5, these arrays are part of the HTTP
transaction and thus are released during its cleanup. Because in this version,
the transaction is part of the stream (in 1.5, streams are still called
sessions), the cleanup is always performed, for HTTP and TCP streams.

In HAProxy 1.6, the HTTP transaction was moved out from the stream and is now
dynamically allocated only when required (becaues of an HTTP proxy or an HTTP
sample fetch). In addition, still in 1.6, the captures arrays were moved from
the HTTP transaction to the stream. This way, it is still possible to capture
elements from TCP rules for a full TCP stream. Unfortunately, the release is
still exclusively performed during the HTTP transaction cleanup. Thus, for a TCP
stream where the HTTP transaction is not required, the TCP captures, if any, are
never released.

Now, all captures are released when the stream is freed. This fixes the memory
leak for TCP streams. For streams with an HTTP transaction, the captures are now
released when the transaction is reset and not systematically during its
cleanup.

This patch must be backported as fas as 1.6.
---

diff --git a/src/http_ana.c b/src/http_ana.c
index cb9308bc8c..6ced37fb39 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -5417,7 +5417,6 @@ void http_init_txn(struct stream *s)
 void http_end_txn(struct stream *s)
 {
 	struct http_txn *txn = s->txn;
-	struct proxy *fe = strm_fe(s);
 
 	/* these ones will have been dynamically allocated */
 	pool_free(pool_head_requri, txn->uri);
@@ -5430,13 +5429,27 @@ void http_end_txn(struct stream *s)
 	txn->srv_cookie = NULL;
 	txn->cli_cookie = NULL;
 
+	if (!LIST_ISEMPTY(&s->vars_txn.head))
+		vars_prune(&s->vars_txn, s->sess, s);
+	if (!LIST_ISEMPTY(&s->vars_reqres.head))
+		vars_prune(&s->vars_reqres, s->sess, s);
+}
+
+/* to be used at the end of a transaction to prepare a new one */
+void http_reset_txn(struct stream *s)
+{
+	struct proxy *fe = strm_fe(s);
+
+	http_end_txn(s);
+	http_init_txn(s);
+
+	/* cleanup and reinit capture arrays, if any */
 	if (s->req_cap) {
 		struct cap_hdr *h;
 		for (h = fe->req_cap; h; h = h->next)
 			pool_free(h->pool, s->req_cap[h->index]);
 		memset(s->req_cap, 0, fe->nb_req_cap * sizeof(void *));
 	}
-
 	if (s->res_cap) {
 		struct cap_hdr *h;
 		for (h = fe->rsp_cap; h; h = h->next)
@@ -5444,18 +5457,6 @@ void http_end_txn(struct stream *s)
 		memset(s->res_cap, 0, fe->nb_rsp_cap * sizeof(void *));
 	}
 
-	if (!LIST_ISEMPTY(&s->vars_txn.head))
-		vars_prune(&s->vars_txn, s->sess, s);
-	if (!LIST_ISEMPTY(&s->vars_reqres.head))
-		vars_prune(&s->vars_reqres, s->sess, s);
-}
-
-/* to be used at the end of a transaction to prepare a new one */
-void http_reset_txn(struct stream *s)
-{
-	http_end_txn(s);
-	http_init_txn(s);
-
 	/* reinitialise the current rule list pointer to NULL. We are sure that
 	 * any rulelist match the NULL pointer.
 	 */
diff --git a/src/stream.c b/src/stream.c
index 8646ce84f5..be9a9fabe3 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -644,6 +644,18 @@ static void stream_free(struct stream *s)
 	flt_stream_release(s, 0);
 
 	if (fe) {
+		if (s->req_cap) {
+			struct cap_hdr *h;
+			for (h = fe->req_cap; h; h = h->next)
+				pool_free(h->pool, s->req_cap[h->index]);
+		}
+
+		if (s->res_cap) {
+			struct cap_hdr *h;
+			for (h = fe->rsp_cap; h; h = h->next)
+				pool_free(h->pool, s->res_cap[h->index]);
+		}
+
 		pool_free(fe->rsp_cap_pool, s->res_cap);
 		pool_free(fe->req_cap_pool, s->req_cap);
 	}