BUG/MEDIUM: fd: Mark a fd as dead on early close

Commit ad082111a04b4387f8aa76e386a2f340f9583bd2 made it so
conn_ctrl_close() will close a fd, if no xprt was created, because
nobody else would do it for us. But this is not entirely true, some code
paths, such as the checks, if connection failed, will take care of
closing the fd already, and doing it in conn_ctrl_close() will lead to a
second close, that may close an unrelated fd.
Fix that by changing the fd to DEAD_FD_MAGIC so that conn_ctrl_close()
knows the fd has already been closed, and don't have to do it.
It would probably be cleaner to just remove those calls to close(), and
always let conn_ctrl_close() do it, but for that we'd have to make sure
conn_ctrl_close() is always called, and that close to the release, I'd
rather go the safer way.

This should be backported to all supported versions, when
ad082111a04b4387f8aa76e386a2f340f9583bd2 has been backported already.

This should fix github issue #3573.

diff --git a/src/proto_sockpair.c b/src/proto_sockpair.c
index 840cd401043beed80855fce39c43f29ea51ada76..fb10fe2bf14f8d173b6fa6c3662fcaabfc34ff38 100644 (file)
--- a/src/proto_sockpair.c
+++ b/src/proto_sockpair.c
@@ -329,6 +329,7 @@ static int sockpair_connect_server(struct connection *conn, int flags)
                ha_alert("socket(): not enough free sockets. Raise -n argument. Giving up.\n");
                close(sv[0]);
                close(sv[1]);
+               conn->handle.fd = DEAD_FD_MAGIC;
                conn->err_code = CO_ER_CONF_FDLIM;
                conn->flags |= CO_FL_ERROR;
                return SF_ERR_PRXCOND; /* it is a configuration limit */
@@ -338,6 +339,7 @@ static int sockpair_connect_server(struct connection *conn, int flags)
                qfprintf(stderr,"Cannot set client socket to non blocking mode.\n");
                close(sv[0]);
                close(sv[1]);
+               conn->handle.fd = DEAD_FD_MAGIC;
                conn->err_code = CO_ER_SOCK_ERR;
                conn->flags |= CO_FL_ERROR;
                return SF_ERR_INTERNAL;
@@ -347,6 +349,7 @@ static int sockpair_connect_server(struct connection *conn, int flags)
                ha_alert("Cannot set CLOEXEC on client socket.\n");
                close(sv[0]);
                close(sv[1]);
+               conn->handle.fd = DEAD_FD_MAGIC;
                conn->err_code = CO_ER_SOCK_ERR;
                conn->flags |= CO_FL_ERROR;
                return SF_ERR_INTERNAL;
@@ -364,6 +367,7 @@ static int sockpair_connect_server(struct connection *conn, int flags)
                ha_alert("socketpair: Cannot transfer the fd %d over sockpair@%d. Giving up.\n", sv[0], dst_fd);
                close(sv[0]);
                close(sv[1]);
+               conn->handle.fd = DEAD_FD_MAGIC;
                conn->err_code = CO_ER_SOCK_ERR;
                conn->flags |= CO_FL_ERROR;
                return SF_ERR_INTERNAL;

diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index 71ce3faa51d1c1d039bb6f36bcb364e94ee4e28f..ec1fe165c7f2ea98de7cbc7f6123dd46d5f180b2 100644 (file)
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -499,6 +499,7 @@ int tcp_connect_server(struct connection *conn, int flags)
                if (unlikely(ret != 0)) {
                        port_range_release_port(src->sport_range, local_port);
                        close(fd);
+                       conn->handle.fd = DEAD_FD_MAGIC;
 
                        if (ret == 1) {
                                ha_alert("Cannot bind to source address before connect() for backend %s. Aborting.\n",
@@ -607,6 +608,7 @@ int tcp_connect_server(struct connection *conn, int flags)
                        qfprintf(stderr,"Connect() failed for backend %s: %s.\n", be->id, msg);
                        port_range_release_port(src ? src->sport_range : NULL, local_port);
                        close(fd);
+                       conn->handle.fd = DEAD_FD_MAGIC;
                        send_log(be, LOG_ERR, "Connect() failed for backend %s: %s.\n", be->id, msg);
                        conn->flags |= CO_FL_ERROR;
                        return SF_ERR_RESOURCE;
@@ -614,6 +616,7 @@ int tcp_connect_server(struct connection *conn, int flags)
                        //qfprintf(stderr,"Connect(): ETIMEDOUT");
                        port_range_release_port(src ? src->sport_range : NULL, local_port);
                        close(fd);
+                       conn->handle.fd = DEAD_FD_MAGIC;
                        conn->err_code = CO_ER_SOCK_ERR;
                        conn->flags |= CO_FL_ERROR;
                        return SF_ERR_SRVTO;
@@ -622,6 +625,7 @@ int tcp_connect_server(struct connection *conn, int flags)
                        //qfprintf(stderr,"Connect(): %d", errno);
                        port_range_release_port(src ? src->sport_range : NULL, local_port);
                        close(fd);
+                       conn->handle.fd = DEAD_FD_MAGIC;
                        conn->err_code = CO_ER_SOCK_ERR;
                        conn->flags |= CO_FL_ERROR;
                        return SF_ERR_SRVCL;

diff --git a/src/proto_uxst.c b/src/proto_uxst.c
index a714167de5e10e4c1979bab368e5a0864dae0326..d90ae4494199547760c44a2db1ac1207016b23ab 100644 (file)
--- a/src/proto_uxst.c
+++ b/src/proto_uxst.c
@@ -364,18 +364,21 @@ static int uxst_connect_server(struct connection *conn, int flags)
 
                        qfprintf(stderr,"Connect() failed for backend %s: %s (%s).\n", be->id, msg, strerror(errno));
                        close(fd);
+                       conn->handle.fd = DEAD_FD_MAGIC;
                        send_log(be, LOG_ERR, "Connect() failed for backend %s: %s (%s).\n", be->id, msg, strerror(errno));
                        conn->flags |= CO_FL_ERROR;
                        return SF_ERR_RESOURCE;
                }
                else if (errno == ETIMEDOUT) {
                        close(fd);
+                       conn->handle.fd = DEAD_FD_MAGIC;
                        conn->err_code = CO_ER_SOCK_ERR;
                        conn->flags |= CO_FL_ERROR;
                        return SF_ERR_SRVTO;
                }
                else {  // (errno == ECONNREFUSED || errno == ENETUNREACH || errno == EACCES || errno == EPERM)
                        close(fd);
+                       conn->handle.fd = DEAD_FD_MAGIC;
                        conn->err_code = CO_ER_SOCK_ERR;
                        conn->flags |= CO_FL_ERROR;
                        return SF_ERR_SRVCL;