]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MAJOR: connection: risk of crash on certain tricky close scenario
authorWilly Tarreau <w@1wt.eu>
Mon, 22 Oct 2012 20:47:55 +0000 (22:47 +0200)
committerWilly Tarreau <w@1wt.eu>
Mon, 22 Oct 2012 20:47:55 +0000 (22:47 +0200)
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 !

src/connection.c

index 732a7eaac36401bb7d42bc747560342e6701c415..08b53ae3e30d3b149dde29adeeaaa9b660bd19f2 100644 (file)
@@ -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.