From: William Lallemand <wlallemand@haproxy.org>
Date: Fri, 28 Jan 2022 20:17:30 +0000 (+0100)
Subject: BUG/MINOR: mworker: fix a FD leak of a sockpair upon a failed reload
X-Git-Tag: v2.6-dev1~25
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=55a921c9147c2d45621964428c86c5ceb9fd31d8;p=thirdparty%2Fhaproxy.git

BUG/MINOR: mworker: fix a FD leak of a sockpair upon a failed reload

When starting HAProxy in master-worker, the master pre-allocate a struct
mworker_proc and do a socketpair() before the configuration parsing. If
the configuration loading failed, the FD are never closed because they
aren't part of listener, they are not even in the fdtab.

This patch fixes the issue by cleaning the mworker_proc structure that
were not asssigned a process, and closing its FDs.

Must be backported as far as 2.0, the srv_drop() only frees the memory
and could be dropped since it's done before an exec().
---

diff --git a/include/haproxy/mworker.h b/include/haproxy/mworker.h
index 0e166ac090..09d6d587e2 100644
--- a/include/haproxy/mworker.h
+++ b/include/haproxy/mworker.h
@@ -42,5 +42,6 @@ int mworker_ext_launch_all(void);
 void mworker_kill_max_reloads(int sig);
 
 void mworker_free_child(struct mworker_proc *);
+void mworker_cleanup_proc();
 
 #endif /* _HAPROXY_MWORKER_H_ */
diff --git a/src/haproxy.c b/src/haproxy.c
index 6013875359..70ce708f3e 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -673,6 +673,7 @@ static void mworker_reexec()
 #endif
 	setenv("HAPROXY_MWORKER_REEXEC", "1", 1);
 
+	mworker_cleanup_proc();
 	mworker_proc_list_to_env(); /* put the children description in the env */
 
 	/* ensure that we close correctly every listeners before reexecuting */
diff --git a/src/mworker.c b/src/mworker.c
index 8b0b06ba02..fa3c1a25d2 100644
--- a/src/mworker.c
+++ b/src/mworker.c
@@ -457,6 +457,36 @@ void mworker_cleanlisteners()
 	}
 }
 
+/* Upon a configuration loading error some mworker_proc and FDs/server were
+ * assigned but the worker was never forked, we must close the FDs and
+ * remove the server
+ */
+void mworker_cleanup_proc()
+{
+	struct mworker_proc *child, *it;
+
+	list_for_each_entry_safe(child, it, &proc_list, list) {
+
+		if (child->pid == -1) {
+			/* Close the socketpair master side.  We don't need to
+			 * close the worker side, because it's stored in the
+			 * GLOBAL cli listener which was supposed to be in the
+			 * worker and which will be closed in
+			 * mworker_cleanlisteners()
+			 */
+			if (child->ipc_fd[0] > -1)
+				close(child->ipc_fd[0]);
+			if (child->srv) {
+				/* only exists if we created a master CLI listener */
+				srv_drop(child->srv);
+			}
+			LIST_DELETE(&child->list);
+			mworker_free_child(child);
+		}
+	}
+}
+
+
 /*  Displays workers and processes  */
 static int cli_io_handler_show_proc(struct appctx *appctx)
 {