From: Vincent Bernat <vincent@bernat.im>
Date: Fri, 20 Oct 2017 06:42:01 +0000 (+0200)
Subject: priv: correctly handle lldpcli exit
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6939cd1e54e5019c9681019a451ab0d97a0d299f;p=thirdparty%2Flldpd.git

priv: correctly handle lldpcli exit

When lldpcli exits, we must acknowledge its death with
`waitpid()`. There were two missing cases:

 - when lldpcli exits before setting the SIG_CHLD signal
 - when privilege separation was not configured

To fix both cases, we ask the OS to reap children for us. We know when
monitored process dies because we are unable to read from pipe (no
need for a signal).

To fix the first case, we just reap any existing dead child after
setting the signal.

Fix #250
---

diff --git a/src/daemon/priv.c b/src/daemon/priv.c
index 92c68d87..16b62657 100644
--- a/src/daemon/priv.c
+++ b/src/daemon/priv.c
@@ -443,7 +443,6 @@ priv_exit_rc_status(int rc, int status) {
 	case 0:
 		/* kill child */
 		kill(monitored, SIGTERM);
-		/* we will receive a sigchld in the future */
 		return;
 	case -1:
 		/* child doesn't exist anymore, we consider this is an error to
@@ -488,21 +487,6 @@ sig_pass_to_chld(int sig)
 	errno = oerrno;
 }
 
-/* if parent gets a SIGCHLD, it will exit */
-static void
-sig_chld(int sig)
-{
-	int status;
-	int rc = waitpid(monitored, &status, WNOHANG);
-	if (rc == 0) {
-		while ((rc = waitpid(-1, &status, WNOHANG)) > 0) {
-			if (rc == monitored) priv_exit_rc_status(rc, status);
-		}
-		return;
-	}
-	priv_exit_rc_status(rc, status);
-}
-
 /* Initialization */
 #define LOCALTIME "/etc/localtime"
 static void
@@ -597,9 +581,9 @@ priv_init(const char *chrootdir, int ctl, uid_t uid, gid_t gid)
 	priv_unprivileged_fd(pair[0]);
 	priv_privileged_fd(pair[1]);
 
+        int status;
 #ifdef ENABLE_PRIVSEP
 	gid_t gidset[1];
-        int status;
 	/* Spawn off monitor */
 	if ((monitored = fork()) < 0)
 		fatal("privsep", "unable to fork monitor");
@@ -652,19 +636,16 @@ priv_init(const char *chrootdir, int ctl, uid_t uid, gid_t gid)
 		sigaction(SIGHUP,  &pass_to_child, NULL);
 		sigaction(SIGINT,  &pass_to_child, NULL);
 		sigaction(SIGQUIT, &pass_to_child, NULL);
-		const struct sigaction child = {
-			.sa_handler = sig_chld,
-			.sa_flags = SA_RESTART
-		};
-		sigaction(SIGCHLD, &child, NULL);
 
-                if (waitpid(monitored, &status, WNOHANG) != 0)
-                        /* Child is already dead */
-                        _exit(1);
+		/* Automatically reap children (including monitored process) */
+		signal(SIGCHLD, SIG_IGN);
+                while (waitpid(-1, &status, WNOHANG) > 0);
 		priv_loop(pair[1], 0);
 		exit(0);
 	}
 #else
+	signal(SIGCHLD, SIG_IGN); /* Automatically reap children (notably lldpcli) */
+	while (waitpid(-1, &status, WNOHANG) > 0);
 	log_warnx("priv", "no privilege separation available");
 	priv_ping();
 #endif