From 6939cd1e54e5019c9681019a451ab0d97a0d299f Mon Sep 17 00:00:00 2001 From: Vincent Bernat Date: Fri, 20 Oct 2017 08:42:01 +0200 Subject: [PATCH] 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 --- src/daemon/priv.c | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) 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 -- 2.39.5