From: Willy Tarreau <w@1wt.eu>
Date: Fri, 25 Sep 2015 18:24:26 +0000 (+0200)
Subject: BUG/MEDIUM: stream-int: avoid double-call to applet->release
X-Git-Tag: v1.6-dev6~68
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=958f0742a20ec1bdf6962169dabff20acd47b77b;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: stream-int: avoid double-call to applet->release

While the SI_ST_DIS state is set *after* doing the close on a connection,
it was set *before* calling release on an applet. Applets have no internal
flags contrary to connections, so they have no way to detect they were
already released. Because of this it happened that applets were closed
twice, once via si_applet_release() and once via si_release_endpoint() at
the end of a transaction. The CLI applet could perform a double free in
this case, though the situation to cause it is quite hard because it
requires that the applet is stuck on output in states that produce very
few data.

In order to solve this, we now assign the SI_ST_DIS state *after* calling
->release, and we refrain from doing so if the state is already assigned.
This makes applets work much more like connections and definitely avoids
this double release.

In the future it might be worth making applets have their own flags like
connections to carry their own state regardless of the stream interface's
state, especially when dealing with connection reuse.

No backport is needed since this issue was caused by the rearchitecture
in 1.6.
---

diff --git a/include/proto/stream_interface.h b/include/proto/stream_interface.h
index 6380f76147..ed8725710a 100644
--- a/include/proto/stream_interface.h
+++ b/include/proto/stream_interface.h
@@ -164,7 +164,7 @@ static inline void si_release_endpoint(struct stream_interface *si)
 		conn_free(conn);
 	}
 	else if ((appctx = objt_appctx(si->end))) {
-		if (appctx->applet->release)
+		if (appctx->applet->release && si->state < SI_ST_DIS)
 			appctx->applet->release(appctx);
 		appctx_free(appctx); /* we share the connection pool */
 	}
@@ -231,7 +231,7 @@ static inline void si_applet_release(struct stream_interface *si)
 	struct appctx *appctx;
 
 	appctx = si_appctx(si);
-	if (appctx && appctx->applet->release)
+	if (appctx && appctx->applet->release && si->state < SI_ST_DIS)
 		appctx->applet->release(appctx);
 }
 
diff --git a/src/stream_interface.c b/src/stream_interface.c
index 2341720b3e..d39d7647ef 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -1406,9 +1406,9 @@ static void stream_int_shutr_applet(struct stream_interface *si)
 		return;
 
 	if (si_oc(si)->flags & CF_SHUTW) {
+		si_applet_release(si);
 		si->state = SI_ST_DIS;
 		si->exp = TICK_ETERNITY;
-		si_applet_release(si);
 	}
 	else if (si->flags & SI_FL_NOHALF) {
 		/* we want to immediately forward this close to the write side */
@@ -1456,8 +1456,8 @@ static void stream_int_shutw_applet(struct stream_interface *si)
 	case SI_ST_QUE:
 	case SI_ST_TAR:
 		/* Note that none of these states may happen with applets */
-		si->state = SI_ST_DIS;
 		si_applet_release(si);
+		si->state = SI_ST_DIS;
 	default:
 		si->flags &= ~(SI_FL_WAIT_ROOM | SI_FL_NOLINGER);
 		ic->flags &= ~CF_SHUTR_NOW;