From: Willy Tarreau <w@1wt.eu>
Date: Mon, 22 Oct 2012 20:47:55 +0000 (+0200)
Subject: BUG/MAJOR: connection: risk of crash on certain tricky close scenario
X-Git-Tag: v1.5-dev13~125
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=153c3cafd7702ccdeca79922670c3fd9473a1d57;p=thirdparty%2Fhaproxy.git

BUG/MAJOR: connection: risk of crash on certain tricky close scenario

In some circumstances, if the connection to the server is aborted while
some data were planned to be sent and the poller reported an ability to
send, then conn_fd_handler() would still call conn->data->send(), causing
the data layer to dereference the now NULL conn->xprt and crash.

So we have to check for conn->xprt validity before calling the data
layer.

This issue was introduced after 1.5-dev12 so it does not need any backport
and does not affect any released version.

Special thanks go to Cristian Ditoiu who once again provided amazing help
to troubleshoot this bug !
---

diff --git a/src/connection.c b/src/connection.c
index 732a7eaac3..08b53ae3e3 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -83,8 +83,12 @@ int conn_fd_handler(int fd)
 	if ((conn->flags & CO_FL_INIT_DATA) && conn->data->init(conn) < 0)
 		return 0;
 
-	/* The data transfer starts here and stops on error and handshakes */
+	/* The data transfer starts here and stops on error and handshakes. Note
+	 * that we must absolutely test conn->xprt at each step in case it suddenly
+	 * changes due to a quick unexpected close().
+	 */
 	if ((fdtab[fd].ev & (FD_POLL_IN | FD_POLL_HUP | FD_POLL_ERR)) &&
+	    conn->xprt &&
 	    !(conn->flags & (CO_FL_WAIT_RD|CO_FL_WAIT_ROOM|CO_FL_ERROR|CO_FL_HANDSHAKE))) {
 		/* force detection of a flag change : it's impossible to have both
 		 * CONNECTED and WAIT_CONN so we're certain to trigger a change.
@@ -94,6 +98,7 @@ int conn_fd_handler(int fd)
 	}
 
 	if ((fdtab[fd].ev & (FD_POLL_OUT | FD_POLL_ERR)) &&
+	    conn->xprt &&
 	    !(conn->flags & (CO_FL_WAIT_WR|CO_FL_WAIT_DATA|CO_FL_ERROR|CO_FL_HANDSHAKE))) {
 		/* force detection of a flag change : it's impossible to have both
 		 * CONNECTED and WAIT_CONN so we're certain to trigger a change.