BUG/MEDIUM: tcp-check: don't call tcpcheck_main() from the I/O handlers!

This function can destroy a socket and create a new one, resulting in a
change of FD on the connection between recv() and send() for example,
which is absolutely not permitted, and can result in various funny games
like polling not being properly updated (or with the flags from a previous
fd) etc.

Let's only call this from the wake() callback which is more tolerant.
Ideally the operations should be made even more reliable by returning
a specific value to indicate that the connection was released and that
another one was created. But this is hasardous for stable releases as
it may reveal other issues.

This fix should be backported to 1.7 and 1.6.

diff --git a/src/checks.c b/src/checks.c
index ad1f5688341c70760bc1588187fc4b417b8a4a43..c1f71970a626efb56005ecec9cafe10202b83791 100644 (file)
--- a/src/checks.c
+++ b/src/checks.c
@@ -757,10 +757,9 @@ static void event_srv_chk_w(struct connection *conn)
        if (!check->type)
                goto out_wakeup;
 
-       if (check->type == PR_O2_TCPCHK_CHK) {
-               tcpcheck_main(conn);
+       /* wake() will take care of calling tcpcheck_main() */
+       if (check->type == PR_O2_TCPCHK_CHK)
                return;
-       }
 
        if (check->bo->o) {
                conn->xprt->snd_buf(conn, check->bo, 0);
@@ -815,10 +814,9 @@ static void event_srv_chk_r(struct connection *conn)
        if (conn->flags & CO_FL_HANDSHAKE)
                return;
 
-       if (check->type == PR_O2_TCPCHK_CHK) {
-               tcpcheck_main(conn);
+       /* wake() will take care of calling tcpcheck_main() */
+       if (check->type == PR_O2_TCPCHK_CHK)
                return;
-       }
 
        /* Warning! Linux returns EAGAIN on SO_ERROR if data are still available
         * but the connection was closed on the remote end. Fortunately, recv still
@@ -1379,6 +1377,10 @@ static int wake_srv_chk(struct connection *conn)
 {
        struct check *check = conn->owner;
 
+       /* we may have to make progress on the TCP checks */
+       if (check->type == PR_O2_TCPCHK_CHK)
+               tcpcheck_main(conn);
+
        if (unlikely(conn->flags & CO_FL_ERROR)) {
                /* We may get error reports bypassing the I/O handlers, typically
                 * the case when sending a pure TCP check which fails, then the I/O