]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: backend: always attach the transport before installing the mux
authorWilly Tarreau <w@1wt.eu>
Fri, 31 Jul 2020 06:39:31 +0000 (08:39 +0200)
committerWilly Tarreau <w@1wt.eu>
Fri, 31 Jul 2020 06:47:58 +0000 (08:47 +0200)
In connect_server(), we can enter in a stupid situation:

  - conn_install_mux_be() is called to install the mux. This one
    subscribes for receiving and quits ;

  - then we discover that a handshake is required on the connection
    (e.g. send-proxy), so xprt_add_hs() is called and subscribes as
    well.

  - we crash in conn_subscribe() which gets a different subscriber.
    And if BUG_ON is disabled, we'd likely lose one event.

Note that it doesn't seem to happen by default, but definitely does
if connect() rightfully performs fd_cant_recv(), so it's a matter of
who does what and in what order.

A simple reproducer consists in adding fd_cant_recv() after fd_cant_send()
in tcp_connect_server() and running it on this config, as discussed in issue

  listen foo
    bind :8181
    mode http
    server srv1 127.0.0.1:8888 send-proxy-v2

The root cause is that xprt_add_hs() installs an xprt layer underneath
the mux without taking over its subscriptions. Ideally if we want to
support this, we'd need to steal the connection's wait_events and
replace them by new ones. But there doesn't seem to be any case where
we're interested in doing this so better simply always install the
transport layer before installing the mux, that's safer and simpler.

This needs to be backported to 2.1 which is constructed the same way
and thus suffers from the same issue, though the code is slightly
different there.

src/backend.c

index d431920d56d5c14f7f1a119aaf8dc5cceb1db836..28ec547f701a18bd3bc6f78c6ed944d3a8b210d1 100644 (file)
@@ -1482,6 +1482,16 @@ int connect_server(struct stream *s)
        }
 #endif /* USE_OPENSSL */
 
+       /* The CO_FL_SEND_PROXY flag may have been set by the connect method,
+        * if so, add our handshake pseudo-XPRT now.
+        */
+       if ((srv_conn->flags & CO_FL_HANDSHAKE)) {
+               if (xprt_add_hs(srv_conn) < 0) {
+                       conn_full_close(srv_conn);
+                       return SF_ERR_INTERNAL;
+               }
+       }
+
        /* We have to defer the mux initialization until after si_connect()
         * has been called, as we need the xprt to have been properly
         * initialized, or any attempt to recv during the mux init may
@@ -1506,16 +1516,6 @@ int connect_server(struct stream *s)
                        session_add_conn(srv_conn->owner, srv_conn, srv_conn->target);
                }
        }
-       /* The CO_FL_SEND_PROXY flag may have been set by the connect method,
-        * if so, add our handshake pseudo-XPRT now.
-        */
-       if ((srv_conn->flags & CO_FL_HANDSHAKE)) {
-               if (xprt_add_hs(srv_conn) < 0) {
-                       conn_full_close(srv_conn);
-                       return SF_ERR_INTERNAL;
-               }
-       }
-
 
 #if USE_OPENSSL && (defined(OPENSSL_IS_BORINGSSL) || (HA_OPENSSL_VERSION_NUMBER >= 0x10101000L))