From 5b19d958506d207247d27bb043033a530af343ab Mon Sep 17 00:00:00 2001 From: William Lallemand Date: Thu, 11 Dec 2025 16:53:18 +0100 Subject: [PATCH] BUG/MEDIUM: mworker/listener: ambiguous use of RX_F_INHERITED with shards The RX_F_INHERITED flag was ambiguous, as it was used to mark both listeners inherited from the parent process and listeners duplicated from another local receiver. This could lead to incorrect behavior concerning socket unbinding and suspension. This commit refactors the handling of inherited listeners by splitting the RX_F_INHERITED flag into two more specific flags: - RX_F_INHERITED_FD: Indicates a listener inherited from the parent process via its file descriptor. These listeners should not be unbound by the master. - RX_F_INHERITED_SOCK: Indicates a listener that shares a socket with another one, either by being inherited from the parent or by being duplicated from another local listener. These listeners should not be suspended or resumed individually. Previously, the sharding code was unconditionally using RX_F_INHERITED when duplicating a file descriptor. In HAProxy versions prior to 3.1, this led to a file descriptor leak for duplicated unix stats sockets in the master process. This would eventually cause the master to crash with a BUG_ON in fd_insert() once the file descriptor limit was reached. This must be backported as far as 3.0. Branches earlier than 3.0 are affected but would need a different patch as the logic is different. --- include/haproxy/receiver-t.h | 3 ++- src/cli.c | 2 +- src/listener.c | 8 +++++++- src/proto_sockpair.c | 6 ------ src/proto_tcp.c | 4 ++-- src/proto_udp.c | 4 ++-- src/sock.c | 2 +- src/sock_inet.c | 8 +------- src/sock_unix.c | 8 +------- 9 files changed, 17 insertions(+), 28 deletions(-) diff --git a/include/haproxy/receiver-t.h b/include/haproxy/receiver-t.h index 90f52aae5..e4e9b2922 100644 --- a/include/haproxy/receiver-t.h +++ b/include/haproxy/receiver-t.h @@ -33,11 +33,12 @@ /* Bit values for receiver->flags */ #define RX_F_BOUND 0x00000001 /* receiver already bound */ -#define RX_F_INHERITED 0x00000002 /* inherited FD from the parent process (fd@) or duped from another local receiver */ +#define RX_F_INHERITED_FD 0x00000002 /* inherited FD from the parent process (fd@) */ #define RX_F_MWORKER 0x00000004 /* keep the FD open in the master but close it in the children */ #define RX_F_MUST_DUP 0x00000008 /* this receiver's fd must be dup() from a reference; ignore socket-level ops here */ #define RX_F_NON_SUSPENDABLE 0x00000010 /* this socket cannot be suspended hence must always be unbound */ #define RX_F_PASS_PKTINFO 0x00000020 /* pass pktinfo in received messages */ +#define RX_F_INHERITED_SOCK 0x00000040 /* inherited sock that could be duped from another local receiver */ /* Bit values for rx_settings->options */ #define RX_O_FOREIGN 0x00000001 /* receives on foreign addresses */ diff --git a/src/cli.c b/src/cli.c index cbcecff43..32365d70a 100644 --- a/src/cli.c +++ b/src/cli.c @@ -3894,7 +3894,7 @@ int mworker_cli_global_proxy_new_listener(struct mworker_proc *proc) list_for_each_entry(l, &bind_conf->listeners, by_bind) { HA_ATOMIC_INC(&unstoppable_jobs); /* it's a sockpair but we don't want to keep the fd in the master */ - l->rx.flags &= ~RX_F_INHERITED; + l->rx.flags &= ~RX_F_INHERITED_FD; global.maxsock++; /* for the listening socket */ } diff --git a/src/listener.c b/src/listener.c index ba1385e47..a86e946ce 100644 --- a/src/listener.c +++ b/src/listener.c @@ -846,7 +846,7 @@ int create_listeners(struct bind_conf *bc, const struct sockaddr_storage *ss, proto->add(proto, l); if (fd != -1) - l->rx.flags |= RX_F_INHERITED; + l->rx.flags |= RX_F_INHERITED_FD|RX_F_INHERITED_SOCK; guid_init(&l->guid); @@ -1844,6 +1844,12 @@ int bind_complete_thread_setup(struct bind_conf *bind_conf, int *err_code) /* it has been allocated already in the previous round */ shard_info_attach(&new_li->rx, ref->rx.shard_info); new_li->rx.flags |= RX_F_MUST_DUP; + /* taking the other one's FD will result in it being marked + * extern and being dup()ed. Let's mark the receiver as + * inherited so that it properly bypasses all second-stage + * setup/unbind and avoids being passed to new processes. + */ + new_li->rx.flags |= ref->rx.flags & RX_F_INHERITED_SOCK; } gmask &= gmask - 1; // drop lowest bit diff --git a/src/proto_sockpair.c b/src/proto_sockpair.c index e9271605f..3c3cd543e 100644 --- a/src/proto_sockpair.c +++ b/src/proto_sockpair.c @@ -151,12 +151,6 @@ int sockpair_bind_receiver(struct receiver *rx, char **errmsg) err |= ERR_RETRYABLE; goto bind_ret_err; } - /* taking the other one's FD will result in it being marked - * extern and being dup()ed. Let's mark the receiver as - * inherited so that it properly bypasses all second-stage - * setup and avoids being passed to new processes. - */ - rx->flags |= RX_F_INHERITED; rx->fd = rx->shard_info->ref->fd; } diff --git a/src/proto_tcp.c b/src/proto_tcp.c index 93115f42f..1f8c8f293 100644 --- a/src/proto_tcp.c +++ b/src/proto_tcp.c @@ -909,7 +909,7 @@ static int tcp_suspend_receiver(struct receiver *rx) * parent process and any possible subsequent worker inheriting it. * Thus we just stop receiving from it. */ - if (rx->flags & RX_F_INHERITED) + if (rx->flags & RX_F_INHERITED_SOCK) goto done; if (connect(rx->fd, &sa, sizeof(sa)) < 0) @@ -945,7 +945,7 @@ static int tcp_resume_receiver(struct receiver *rx) if (rx->fd < 0) return 0; - if ((rx->flags & RX_F_INHERITED) || listen(rx->fd, listener_backlog(l)) == 0) { + if ((rx->flags & RX_F_INHERITED_SOCK) || listen(rx->fd, listener_backlog(l)) == 0) { fd_want_recv(l->rx.fd); return 1; } diff --git a/src/proto_udp.c b/src/proto_udp.c index bedc6f6cb..764124116 100644 --- a/src/proto_udp.c +++ b/src/proto_udp.c @@ -220,7 +220,7 @@ int udp_suspend_receiver(struct receiver *rx) /* we never do that with a shared FD otherwise we'd break it in the * parent process and any possible subsequent worker inheriting it. */ - if (rx->flags & RX_F_INHERITED) + if (rx->flags & RX_F_INHERITED_SOCK) goto done; if (getsockname(rx->fd, (struct sockaddr *)&ss, &len) < 0) @@ -248,7 +248,7 @@ int udp_resume_receiver(struct receiver *rx) if (rx->fd < 0) return 0; - if (!(rx->flags & RX_F_INHERITED) && connect(rx->fd, &sa, sizeof(sa)) < 0) + if (!(rx->flags & RX_F_INHERITED_SOCK) && connect(rx->fd, &sa, sizeof(sa)) < 0) return -1; fd_want_recv(rx->fd); diff --git a/src/sock.c b/src/sock.c index 3644a1a91..bda07024e 100644 --- a/src/sock.c +++ b/src/sock.c @@ -381,7 +381,7 @@ void sock_unbind(struct receiver *rx) return; if (!stopping && master && - rx->flags & RX_F_INHERITED) + rx->flags & RX_F_INHERITED_FD) return; rx->flags &= ~RX_F_BOUND; diff --git a/src/sock_inet.c b/src/sock_inet.c index 4ef77f001..342f3ac0f 100644 --- a/src/sock_inet.c +++ b/src/sock_inet.c @@ -327,12 +327,6 @@ int sock_inet_bind_receiver(struct receiver *rx, char **errmsg) err |= ERR_RETRYABLE; goto bind_ret_err; } - /* taking the other one's FD will result in it being marked - * extern and being dup()ed. Let's mark the receiver as - * inherited so that it properly bypasses all second-stage - * setup and avoids being passed to new processes. - */ - rx->flags |= RX_F_INHERITED; rx->fd = rx->shard_info->ref->fd; } @@ -467,7 +461,7 @@ int sock_inet_bind_receiver(struct receiver *rx, char **errmsg) fd_insert(fd, rx->owner, rx->iocb, rx->bind_tgroup, rx->bind_thread); /* for now, all regularly bound TCP listeners are exportable */ - if (!(rx->flags & RX_F_INHERITED)) + if (!(rx->flags & (RX_F_INHERITED_FD|RX_F_INHERITED_SOCK))) HA_ATOMIC_OR(&fdtab[fd].state, FD_EXPORTED); bind_return: diff --git a/src/sock_unix.c b/src/sock_unix.c index 0d00a15a4..36d60fab3 100644 --- a/src/sock_unix.c +++ b/src/sock_unix.c @@ -237,12 +237,6 @@ int sock_unix_bind_receiver(struct receiver *rx, char **errmsg) err |= ERR_RETRYABLE; goto bind_ret_err; } - /* taking the other one's FD will result in it being marked - * extern and being dup()ed. Let's mark the receiver as - * inherited so that it properly bypasses all second-stage - * setup and avoids being passed to new processes. - */ - rx->flags |= RX_F_INHERITED; rx->fd = rx->shard_info->ref->fd; } @@ -418,7 +412,7 @@ int sock_unix_bind_receiver(struct receiver *rx, char **errmsg) fd_insert(fd, rx->owner, rx->iocb, rx->bind_tgroup, rx->bind_thread); /* for now, all regularly bound TCP listeners are exportable */ - if (!(rx->flags & RX_F_INHERITED)) + if (!(rx->flags & (RX_F_INHERITED_FD|RX_F_INHERITED_SOCK))) HA_ATOMIC_OR(&fdtab[fd].state, FD_EXPORTED); return err;