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

@@ -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 */

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 */
 	}
 

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

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;
 	}
 

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;
 	}

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);

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;

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:

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;