From 5f713c03bed6424fee7dc36fbb00efe368a7e95b Mon Sep 17 00:00:00 2001 From: Valentine Krasnobaeva Date: Tue, 21 May 2024 19:24:31 +0200 Subject: [PATCH] BUG/MEDIUM: proto: fix fd leak in _connect_server This fixes the fd leak, introduced in the commit d3fc982cd788 ("MEDIUM: proto: make common fd checks in sock_create_server_socket"). Initially sock_create_server_socket() was designed to return only created socket FD or -1. Its callers from upper protocol layers were required to test the returned errno and were required then to apply different configuration related checks to obtained positive sock_fd. A lot of this code was duplicated among protocols implementations. The new refactored version of sock_create_server_socket() gathers in one place all duplicated checks, but in order to be complient with upper protocol layers, it needs the 3rd parameter: 'stream_err', in which it sets the Stream Error Flag for upper levels, if the obtained sock_fd has passed all additional checks. No backport needed since this was introduced in 3.0-dev10. --- include/haproxy/sock.h | 2 +- src/proto_quic.c | 9 +++++---- src/proto_tcp.c | 9 +++++---- src/proto_uxst.c | 9 +++++---- src/sock.c | 20 ++++++++++++++------ 5 files changed, 30 insertions(+), 19 deletions(-) diff --git a/include/haproxy/sock.h b/include/haproxy/sock.h index ba8e9d021d..017e0ad1d6 100644 --- a/include/haproxy/sock.h +++ b/include/haproxy/sock.h @@ -30,7 +30,7 @@ #include #include -int sock_create_server_socket(struct connection *conn, struct proxy *be); +int sock_create_server_socket(struct connection *conn, struct proxy *be, int *stream_err); void sock_enable(struct receiver *rx); void sock_disable(struct receiver *rx); void sock_unbind(struct receiver *rx); diff --git a/src/proto_quic.c b/src/proto_quic.c index c616f14e8e..0c4d6558a4 100644 --- a/src/proto_quic.c +++ b/src/proto_quic.c @@ -277,7 +277,7 @@ int quic_bind_socket(int fd, int flags, struct sockaddr_storage *local, struct s int quic_connect_server(struct connection *conn, int flags) { - int fd; + int fd, stream_err; struct server *srv; struct proxy *be; struct conn_src *src; @@ -302,9 +302,10 @@ int quic_connect_server(struct connection *conn, int flags) } /* perform common checks on obtained socket FD, return appropriate Stream Error Flag in case of failure */ - fd = conn->handle.fd = sock_create_server_socket(conn, be); - if (fd & SF_ERR_MASK) - return fd; + fd = conn->handle.fd = sock_create_server_socket(conn, be, &stream_err); + if (fd == -1) { + return stream_err; + } /* FD is ok, perform protocol specific settings */ /* allow specific binding : diff --git a/src/proto_tcp.c b/src/proto_tcp.c index b1a369871c..4e6173c2ee 100644 --- a/src/proto_tcp.c +++ b/src/proto_tcp.c @@ -265,7 +265,7 @@ int tcp_bind_socket(int fd, int flags, struct sockaddr_storage *local, struct so int tcp_connect_server(struct connection *conn, int flags) { - int fd; + int fd, stream_err; struct server *srv; struct proxy *be; struct conn_src *src; @@ -301,9 +301,10 @@ int tcp_connect_server(struct connection *conn, int flags) /* perform common checks on obtained socket FD, return appropriate Stream Error Flag in case of failure */ - fd = conn->handle.fd = sock_create_server_socket(conn, be); - if ((fd & SF_ERR_MASK) == fd) - return fd; + fd = conn->handle.fd = sock_create_server_socket(conn, be, &stream_err); + if (fd == -1) { + return stream_err; + } /* FD is OK, continue with protocol specific settings */ if (be->options & PR_O_TCP_SRV_KA) { diff --git a/src/proto_uxst.c b/src/proto_uxst.c index 45953d3b1f..94d9170314 100644 --- a/src/proto_uxst.c +++ b/src/proto_uxst.c @@ -219,7 +219,7 @@ static int uxst_suspend_receiver(struct receiver *rx) */ static int uxst_connect_server(struct connection *conn, int flags) { - int fd; + int fd, stream_err; struct server *srv; struct proxy *be; @@ -240,9 +240,10 @@ static int uxst_connect_server(struct connection *conn, int flags) } /* perform common checks on obtained socket FD, return appropriate Stream Error Flag in case of failure */ - fd = conn->handle.fd = sock_create_server_socket(conn, be); - if (fd & SF_ERR_MASK) - return fd; + fd = conn->handle.fd = sock_create_server_socket(conn, be, &stream_err); + if (fd == -1) { + return stream_err; + } /* FD is ok, continue with protocol specific settings */ if (global.tune.server_sndbuf) diff --git a/src/sock.c b/src/sock.c index 4f2ba1a761..3961f4f926 100644 --- a/src/sock.c +++ b/src/sock.c @@ -256,9 +256,11 @@ static int sock_handle_system_err(struct connection *conn, struct proxy *be) * using the configured namespace if needed, or the one passed by the proxy * protocol if required to do so. It then calls socket() or socketat(). On * success, checks if mark or tos sockopts need to be set on the file handle. - * Returns the FD or error code. + * Returns backend connection socket FD on success, stream_err flag needed by + * upper level is set as SF_ERR_NONE; -1 on failure, stream_err is set to + * appropriate value. */ -int sock_create_server_socket(struct connection *conn, struct proxy *be) +int sock_create_server_socket(struct connection *conn, struct proxy *be, int *stream_err) { const struct netns_entry *ns = NULL; int sock_fd; @@ -275,7 +277,9 @@ int sock_create_server_socket(struct connection *conn, struct proxy *be) /* at first, handle common to all proto families system limits and permission related errors */ if (sock_fd == -1) { - return sock_handle_system_err(conn, be); + *stream_err = sock_handle_system_err(conn, be); + + return -1; } /* now perform some runtime condition checks */ @@ -288,8 +292,9 @@ int sock_create_server_socket(struct connection *conn, struct proxy *be) close(sock_fd); conn->err_code = CO_ER_CONF_FDLIM; conn->flags |= CO_FL_ERROR; + *stream_err = SF_ERR_PRXCOND; /* it is a configuration limit */ - return SF_ERR_PRXCOND; /* it is a configuration limit */ + return -1; } if (fd_set_nonblock(sock_fd) == -1 || @@ -299,8 +304,9 @@ int sock_create_server_socket(struct connection *conn, struct proxy *be) close(sock_fd); conn->err_code = CO_ER_SOCK_ERR; conn->flags |= CO_FL_ERROR; + *stream_err = SF_ERR_INTERNAL; - return SF_ERR_INTERNAL; + return -1; } if (master == 1 && fd_set_cloexec(sock_fd) == -1) { @@ -309,8 +315,9 @@ int sock_create_server_socket(struct connection *conn, struct proxy *be) close(sock_fd); conn->err_code = CO_ER_SOCK_ERR; conn->flags |= CO_FL_ERROR; + *stream_err = SF_ERR_INTERNAL; - return SF_ERR_INTERNAL; + return -1; } if (conn->flags & CO_FL_OPT_MARK) @@ -318,6 +325,7 @@ int sock_create_server_socket(struct connection *conn, struct proxy *be) if (conn->flags & CO_FL_OPT_TOS) sock_set_tos(sock_fd, conn->dst, conn->tos); + stream_err = SF_ERR_NONE; return sock_fd; }