From 3284a7325a4c5b7382a13b0fa9c98409168368fe Mon Sep 17 00:00:00 2001
From: Christian Hopps <chopps@labn.net>
Date: Tue, 23 Jul 2024 17:42:07 -0400
Subject: [PATCH] lib: mgmtd: fix too early daemon detach of mgmtd

Correct FRR startup counts on a daemon's vty socket to be open when the
parent process exits. The parent process waits for `frr_check_detach()`
to be called by the child before exiting. The problem is when the
`FRR_MANUAL_VTY_START` flag is set the vty socket was not opened but
`frr_check_detach()` was called anyway.

Instead add a bool option for `frr_check_detach()` to be called when the
socket is opened with `frr_vty_serv_start()`, and do so when "manually"
calling said function (i.e., when FRR_MANUAL_VTY_START is set).

The `FRR_MANUAL_VTY_START` flag is only set by mgmtd. The reason we
wait to open the vty socket is so that mgmtd can parse the various
daemon specific config files it has taken over, after the event loop has
started, but before we receive any possible new config from `vtysh`.

fixes #16362

Signed-off-by: Christian Hopps <chopps@labn.net>
(cherry picked from commit be9a6fc0ea8180a4aaa558c5402ea543427e2e7e)
---
 lib/libfrr.c | 30 +++++++++++++++++-------------
 lib/libfrr.h |  2 +-
 lib/vty.c    |  2 +-
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/lib/libfrr.c b/lib/libfrr.c
index 2861ebe0b933..cb295f7e1b93 100644
--- a/lib/libfrr.c
+++ b/lib/libfrr.c
@@ -1035,7 +1035,17 @@ void frr_config_fork(void)
 	zlog_tls_buffer_init();
 }
 
-void frr_vty_serv_start(void)
+static void frr_check_detach(void)
+{
+	if (nodetach_term || nodetach_daemon)
+		return;
+
+	if (daemon_ctl_sock != -1)
+		close(daemon_ctl_sock);
+	daemon_ctl_sock = -1;
+}
+
+void frr_vty_serv_start(bool check_detach)
 {
 	/* allow explicit override of vty_path in the future
 	 * (not currently set anywhere) */
@@ -1058,6 +1068,9 @@ void frr_vty_serv_start(void)
 	}
 
 	vty_serv_start(di->vty_addr, di->vty_port, di->vty_path);
+
+	if (check_detach)
+		frr_check_detach();
 }
 
 void frr_vty_serv_stop(void)
@@ -1068,16 +1081,6 @@ void frr_vty_serv_stop(void)
 		unlink(di->vty_path);
 }
 
-static void frr_check_detach(void)
-{
-	if (nodetach_term || nodetach_daemon)
-		return;
-
-	if (daemon_ctl_sock != -1)
-		close(daemon_ctl_sock);
-	daemon_ctl_sock = -1;
-}
-
 static void frr_terminal_close(int isexit)
 {
 	int nullfd;
@@ -1163,7 +1166,7 @@ void frr_run(struct event_loop *master)
 	char instanceinfo[64] = "";
 
 	if (!(di->flags & FRR_MANUAL_VTY_START))
-		frr_vty_serv_start();
+		frr_vty_serv_start(false);
 
 	if (di->instance)
 		snprintf(instanceinfo, sizeof(instanceinfo), "instance %u ",
@@ -1201,7 +1204,8 @@ void frr_run(struct event_loop *master)
 			close(nullfd);
 		}
 
-		frr_check_detach();
+		if (!(di->flags & FRR_MANUAL_VTY_START))
+			frr_check_detach();
 	}
 
 	/* end fixed stderr startup logging */
diff --git a/lib/libfrr.h b/lib/libfrr.h
index ee436d9f8f88..8de881d5da0f 100644
--- a/lib/libfrr.h
+++ b/lib/libfrr.h
@@ -200,7 +200,7 @@ extern void frr_config_fork(void);
 
 extern void frr_run(struct event_loop *master);
 extern void frr_detach(void);
-extern void frr_vty_serv_start(void);
+extern void frr_vty_serv_start(bool check_detach);
 extern void frr_vty_serv_stop(void);
 
 extern bool frr_zclient_addr(struct sockaddr_storage *sa, socklen_t *sa_len,
diff --git a/lib/vty.c b/lib/vty.c
index 22858b937f5b..cec8dd64b860 100644
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -3486,7 +3486,7 @@ static void vty_mgmt_server_connected(struct mgmt_fe_client *client,
 
 	/* Start or stop listening for vty connections */
 	if (connected)
-		frr_vty_serv_start();
+		frr_vty_serv_start(true);
 	else
 		frr_vty_serv_stop();
 }