RepoMirrors
/
haproxy
mirror of http://git.haproxy.org/git/haproxy.git/
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Willy Tarreau 2023-02-09 17:53:41 +01:00
parent eb3f26d5a0
commit db991c2658
1 changed files with 11 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/session.c
View File

@ -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);