BUG/MEDIUM: quic: fix crash when "option nolinger" is set in the frontend

Commit 0aba11e9e ("MINOR: quic: remove unnecessary quic_session_accept()")
overlooked one problem, in session_accept_fd() at the end, there's a bunch
of FD-specific stuff that either sets up or resets the socket at the TCP
level. The tests are mostly performed for AF_INET/AF_INET6 families but
they're only for one part (i.e. to avoid setting up TCP options on UNIX
sockets). Other pieces continue to configure the socket regardless of its
family. All of this directly acts on the FD, which is not correct since
the FD is not valid here, it corresponds to the QUIC handle. The issue
is much more visible when "option nolinger" is enabled in the frontend,
because the access to fdatb[cfd].state immediately crashes on the first
connection, as can be seen in github issue #2030.

This patch bypasses this setup for FD-less connections, such as QUIC.
However some of them could definitely be relevant to the QUIC stack, or
even to UNIX sockets sometimes. A better long-term solution would consist
in implementing a setsockopt() equivalent at the protocol layer that would
be used to configure the socket, either the FD or the QUIC conn depending
on the case. Some of them would not always be implemented but that would
allow to unify all this code.

This fix must be backported everywhere the commit above is backported,
namely 2.6 and 2.7.

Thanks to github user @twomoses for the nicely detailed report.

diff --git a/src/session.c b/src/session.c
index a6cca8243274c079fbf9e1a98ca533467be294f9..c53141d9eb4cb38e576bde7943c084ffc6341403 100644 (file)
--- a/src/session.c
+++ b/src/session.c
@@ -192,7 +192,8 @@ int session_accept_fd(struct connection *cli_conn)
         */
        if (!LIST_ISEMPTY(&p->tcp_req.l4_rules) && !tcp_exec_l4_rules(sess)) {
                /* let's do a no-linger now to close with a single RST. */
-               setsockopt(cfd, SOL_SOCKET, SO_LINGER, (struct linger *) &nolinger, sizeof(struct linger));
+               if (!(cli_conn->flags & CO_FL_FDLESS))
+                       setsockopt(cfd, SOL_SOCKET, SO_LINGER, (struct linger *) &nolinger, sizeof(struct linger));
                ret = 0; /* successful termination */
                goto out_free_sess;
        }
@@ -200,6 +201,12 @@ int session_accept_fd(struct connection *cli_conn)
        if (conn_xprt_start(cli_conn) < 0)
                goto out_free_sess;
 
+       /* FIXME/WTA: we should implement the setsockopt() calls at the proto
+        * level instead and let non-inet protocols implement their own equivalent.
+        */
+       if (cli_conn->flags & CO_FL_FDLESS)
+               goto skip_fd_setup;
+
        /* Adjust some socket options */
        if (l->rx.addr.ss_family == AF_INET || l->rx.addr.ss_family == AF_INET6) {
                setsockopt(cfd, IPPROTO_TCP, TCP_NODELAY, (char *) &one, sizeof(one));
@@ -245,6 +252,7 @@ int session_accept_fd(struct connection *cli_conn)
        if (global.tune.client_rcvbuf)
                setsockopt(cfd, SOL_SOCKET, SO_RCVBUF, &global.tune.client_rcvbuf, sizeof(global.tune.client_rcvbuf));
 
+ skip_fd_setup:
        /* OK, now either we have a pending handshake to execute with and then
         * we must return to the I/O layer, or we can proceed with the end of
         * the stream initialization. In case of handshake, we also set the I/O
@@ -292,7 +300,8 @@ int session_accept_fd(struct connection *cli_conn)
 
  out_free_conn:
        if (ret < 0 && l->bind_conf->xprt == xprt_get(XPRT_RAW) &&
-           p->mode == PR_MODE_HTTP && l->bind_conf->mux_proto == NULL) {
+           p->mode == PR_MODE_HTTP && l->bind_conf->mux_proto == NULL &&
+           !(cli_conn->flags & CO_FL_FDLESS)) {
                /* critical error, no more memory, try to emit a 500 response */
                send(cfd, http_err_msgs[HTTP_ERR_500], strlen(http_err_msgs[HTTP_ERR_500]),
                     MSG_DONTWAIT|MSG_NOSIGNAL);