BUG/MEDIUM: proto: fix fd leak in <proto>_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.

diff --git a/include/haproxy/sock.h b/include/haproxy/sock.h
index ba8e9d021dfa233a71fd0df2014df67b8e1fb94e..017e0ad1d65b44c8475082878ced85c46b7a71ac 100644 (file)
--- a/include/haproxy/sock.h
+++ b/include/haproxy/sock.h
@@ -30,7 +30,7 @@
 #include <haproxy/listener-t.h>
 #include <haproxy/sock-t.h>
 
-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 c616f14e8e2028d21833088913a5d413f82f06b0..0c4d6558a43565a97638ce9abf8a151ee5e49d56 100644 (file)
--- 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 b1a369871c21721fce6c1268db276985f32ff939..4e6173c2ee06abb3e7fb1c71d23759934d9c03f7 100644 (file)
--- 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 45953d3b1f4affe10dc81a9c3d27dcc69c0ad631..94d9170314419e44b5617793799d73ab34eda583 100644 (file)
--- 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 4f2ba1a761ab9a099458c73ebf4413f7f71a57ec..3961f4f926917e2dcd5869fe96d443d855b0801f 100644 (file)
--- 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;
 }