From: Christopher Faulet <cfaulet@haproxy.com>
Date: Mon, 8 Apr 2019 08:52:21 +0000 (+0200)
Subject: MEDIUM: muxes: Be prepared to don't own connection during the release
X-Git-Tag: v2.0-dev3~290
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=39a96ee16eeec51774f9f52a783cf624a0de4ccb;p=thirdparty%2Fhaproxy.git

MEDIUM: muxes: Be prepared to don't own connection during the release

This happens during mux upgrades. In such case, when the destroy() callback is
called, the connection points to a different mux's context than the one passed
to the callback. It means the connection is owned by another mux. The old mux is
then released but the connection is not closed.
---

diff --git a/src/mux_h1.c b/src/mux_h1.c
index 43b062bbb5..ef9dd01dce 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -445,6 +445,10 @@ static void h1_release(struct h1c *h1c)
 {
 	struct connection *conn = h1c->conn;
 
+	/* The connection was attached to another mux */
+	if (conn && conn->ctx != h1c)
+		conn = NULL;
+
 	if (h1c) {
 		if (!LIST_ISEMPTY(&h1c->buf_wait.list)) {
 			HA_SPIN_LOCK(BUF_WQ_LOCK, &buffer_wq_lock);
@@ -466,20 +470,20 @@ static void h1_release(struct h1c *h1c)
 			tasklet_free(h1c->wait_event.task);
 
 		h1s_destroy(h1c->h1s);
-		if (h1c->wait_event.events != 0)
-			conn->xprt->unsubscribe(conn, h1c->wait_event.events,
-			    &h1c->wait_event);
 		pool_free(pool_head_h1c, h1c);
 	}
 
-	conn->mux = NULL;
-	conn->ctx = NULL;
+	if (conn) {
+		conn->mux = NULL;
+		conn->ctx = NULL;
 
-	conn_stop_tracking(conn);
-	conn_full_close(conn);
-	if (conn->destroy_cb)
-		conn->destroy_cb(conn);
-	conn_free(conn);
+		conn_force_unsubscribe(conn);
+		conn_stop_tracking(conn);
+		conn_full_close(conn);
+		if (conn->destroy_cb)
+			conn->destroy_cb(conn);
+		conn_free(conn);
+	}
 }
 
 /******************************************************/
@@ -1988,7 +1992,7 @@ static void h1_destroy(void *ctx)
 {
 	struct h1c *h1c = ctx;
 
-	if (!h1c->h1s)
+	if (!h1c->h1s || !h1c->conn || h1c->conn->ctx != h1c)
 		h1_release(h1c);
 }
 
diff --git a/src/mux_h2.c b/src/mux_h2.c
index 32e1fc1ab1..48876ee20d 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -621,6 +621,11 @@ static void h2_release(struct h2c *h2c)
 {
 	struct connection *conn = h2c->conn;
 
+	/* The connection was attached to another mux (unexpected but safer to
+	 * check) */
+	if (conn && conn->ctx != h2c)
+		conn = NULL;
+
 	if (h2c) {
 		hpack_dht_free(h2c->ddht);
 
@@ -638,21 +643,21 @@ static void h2_release(struct h2c *h2c)
 		}
 		if (h2c->wait_event.task)
 			tasklet_free(h2c->wait_event.task);
-		if (h2c->wait_event.events != 0)
-			conn->xprt->unsubscribe(conn, h2c->wait_event.events,
-			    &h2c->wait_event);
 
 		pool_free(pool_head_h2c, h2c);
 	}
 
-	conn->mux = NULL;
-	conn->ctx = NULL;
+	if (conn) {
+		conn->mux = NULL;
+		conn->ctx = NULL;
 
-	conn_stop_tracking(conn);
-	conn_full_close(conn);
-	if (conn->destroy_cb)
-		conn->destroy_cb(conn);
-	conn_free(conn);
+		conn_force_unsubscribe(conn);
+		conn_stop_tracking(conn);
+		conn_full_close(conn);
+		if (conn->destroy_cb)
+			conn->destroy_cb(conn);
+		conn_free(conn);
+	}
 }
 
 
@@ -2989,7 +2994,7 @@ static void h2_destroy(void *ctx)
 {
 	struct h2c *h2c = ctx;
 
-	if (eb_is_empty(&h2c->streams_by_id))
+	if (eb_is_empty(&h2c->streams_by_id) || !h2c->conn || h2c->conn->ctx != h2c)
 		h2_release(h2c);
 }
 
diff --git a/src/mux_pt.c b/src/mux_pt.c
index 6fb9f2f56e..82ba124b11 100644
--- a/src/mux_pt.c
+++ b/src/mux_pt.c
@@ -28,17 +28,23 @@ static void mux_pt_destroy(struct mux_pt_ctx *ctx)
 {
 	struct connection *conn = ctx->conn;
 
-	conn_stop_tracking(conn);
-	conn_full_close(conn);
-	tasklet_free(ctx->wait_event.task);
-	conn->mux = NULL;
-	conn->ctx = NULL;
-	if (conn->destroy_cb)
-		conn->destroy_cb(conn);
-	/* We don't bother unsubscribing here, as we're about to destroy
-	 * both the connection and the mux_pt_ctx
-	 */
-	conn_free(conn);
+	/* The connection was attached to another mux */
+	if (conn && conn->ctx != ctx)
+		conn = NULL;
+
+	if (conn) {
+		conn_stop_tracking(conn);
+		conn_full_close(conn);
+		tasklet_free(ctx->wait_event.task);
+		conn->mux = NULL;
+		conn->ctx = NULL;
+		if (conn->destroy_cb)
+			conn->destroy_cb(conn);
+		/* We don't bother unsubscribing here, as we're about to destroy
+		 * both the connection and the mux_pt_ctx
+		 */
+		conn_free(conn);
+	}
 	pool_free(pool_head_pt_ctx, ctx);
 }
 
@@ -172,7 +178,7 @@ static void mux_pt_destroy_meth(void *ctx)
 {
 	struct mux_pt_ctx *pt = ctx;
 
-	if (!(pt->cs))
+	if (!(pt->cs) || !(pt->conn) || pt->conn->ctx != pt)
 		mux_pt_destroy(pt);
 }