From a3b17563e16ba0dd1dd9a617424fe198d7687bac Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 31 Jul 2020 08:39:31 +0200 Subject: [PATCH] BUG/MEDIUM: backend: always attach the transport before installing the mux 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 | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/backend.c b/src/backend.c index d431920d5..28ec547f7 100644 --- a/src/backend.c +++ b/src/backend.c @@ -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))