BUG/MINOR: session: fix theoretical risk of memleak in session_accept_fd()

Andrew Suffield reported in issue #1596 that we've had a bug in
session_accept_fd() since 2.4 with commit 1b3c931bf ("MEDIUM:
connections: Introduce a new XPRT method, start().") where an error
label is wrong and may cause the leak of the freshly allocated session
in case conn_xprt_start() returns < 0.

The code was checked there and the only two transport layers available
at this point are raw_sock and ssl_sock. The former doesn't provide a
->start() method hence conn_xprt_start() will always return zero. The
second does provide such a function, but it may only return <0 if the
underlying transport (raw_sock) has such a method and fails, which is
thus not the case.

So fortunately it is not possible to trigger this leak.

The patch above also touched the accept code in quic_sock() which was
mostly a plain copy of the session code, but there the move didn't
have this impact, and since then it was simplified and the next change
moved it to its final destination with the proper error label.

This should be backported as far as 2.4 as a long-term safety measure
(e.g. if in the future we have a reason for making conn_xprt_start()
to start failing), but will not have any positive nor negative effect
in the short term.
This commit is contained in:
Willy Tarreau 2022-03-11 07:25:11 +01:00
parent 0657b93385
commit d2985f3cec

View File

@ -188,7 +188,7 @@ int session_accept_fd(struct connection *cli_conn)
}
/* TCP rules may flag the connection as needing proxy protocol, now that it's done we can start ourxprt */
if (conn_xprt_start(cli_conn) < 0)
goto out_free_conn;
goto out_free_sess;
/* Adjust some socket options */
if (l->rx.addr.ss_family == AF_INET || l->rx.addr.ss_family == AF_INET6) {