privsep: Improve the race to exit

Each process should now cleanly wait for child processes to exit.
They should only exit when no children left.

There is still no way to cleanly log the privilged process exiting
as well as the manager process as the manager needs the
privilged process to log.

Now, at least, dhcpcd should alway say it's exited.

diff --git a/src/privsep-root.c b/src/privsep-root.c
index a8290a4063befc2d2ad62a670e4dd01bd87806ff..28b8f59f4693dbfbf110d2b823903a587534905f 100644 (file)
--- a/src/privsep-root.c
+++ b/src/privsep-root.c
@@ -784,7 +784,7 @@ ps_root_signalcb(int sig, void *arg)
 
        if (!(ctx->options & DHCPCD_EXITING))
                return;
-       if (!(PS_WAITING_FOR_PROCESSES(ctx)))
+       if (!(ps_waitforprocs(ctx)))
                eloop_exit(ctx->ps_eloop, EXIT_SUCCESS);
 }
 
@@ -924,22 +924,34 @@ ps_root_start(struct dhcpcd_ctx *ctx)
 int
 ps_root_stop(struct dhcpcd_ctx *ctx)
 {
-
-       /* If we are the root process, ensure the log fd is fully drained. */
-       if (ctx->options & DHCPCD_PRIVSEPROOT && ctx->ps_log_fd != -1) {
-               do {
-                       ;
-               } while (logreadfd(ctx->ps_log_fd) != -1);
-       }
+       struct ps_process *psp = ctx->ps_root;
 
        if (!(ctx->options & DHCPCD_PRIVSEP) ||
-           ctx->options & DHCPCD_FORKED ||
            ctx->eloop == NULL)
                return 0;
 
-       if (ps_stopprocess(ctx->ps_root) == -1)
+       /* If we are the root process then remove the pidfile */
+       if (ctx->options & DHCPCD_PRIVSEPROOT) {
+               if (unlink(ctx->pidfile) == -1)
+                       logerr("%s: unlink: %s", __func__, ctx->pidfile);
+       }
+
+       /* Only the manager process gets past this point. */
+       if (ctx->options & DHCPCD_FORKED)
+               return 0;
+
+       /* We cannot log the root process exited before we
+        * log dhcpcd exits because the latter requires the former.
+        * So we just log the intent to exit.
+        * Even sending this will be a race to exit. */
+       logdebugx("%s%s%s will exit from PID %d",
+           psp->psp_ifname,
+           psp->psp_ifname[0] != '\0' ? ": " : "",
+           psp->psp_name, psp->psp_pid);
+
+       if (ps_stopprocess(psp) == -1)
                return -1;
-       ctx->ps_root = NULL;
+
        return ps_stopwait(ctx);
 }
 

diff --git a/src/privsep.c b/src/privsep.c
index 5a437ae3ae040e7969ddc1973c28be5237025cf3..3db9f4fcf0ec4fadb88cb8e674d7e39b1b608b86 100644 (file)
--- a/src/privsep.c
+++ b/src/privsep.c
@@ -337,7 +337,7 @@ ps_processhangup(void *arg, unsigned short events)
 
        if (!(ctx->options & DHCPCD_EXITING))
                return;
-       if (!(PS_WAITING_FOR_PROCESSES(ctx)))
+       if (!(ps_waitforprocs(ctx)))
                eloop_exit(ctx->ps_eloop, EXIT_SUCCESS);
 }
 #endif
@@ -715,55 +715,53 @@ ps_stop(struct dhcpcd_ctx *ctx)
        return ret;
 }
 
+bool
+ps_waitforprocs(struct dhcpcd_ctx *ctx)
+{
+       struct ps_process *psp = TAILQ_FIRST(&ctx->ps_processes);
+
+       if (psp == NULL)
+               return false;
+
+       /* Different processes */
+       if (psp != TAILQ_LAST(&ctx->ps_processes, ps_process_head))
+               return true;
+
+       return !psp->psp_started;
+}
+
 int
 ps_stopwait(struct dhcpcd_ctx *ctx)
 {
        int error = EXIT_SUCCESS;
 
-       if (ctx->ps_eloop != NULL && PS_WAITING_FOR_PROCESSES(ctx)) {
-               int waited;
+       if (ctx->ps_eloop == NULL || !ps_waitforprocs(ctx))
+               return 0;
 
-               ctx->options |= DHCPCD_EXITING;
-               if (eloop_timeout_add_sec(ctx->ps_eloop, PS_PROCESS_TIMEOUT,
-                   ps_process_timeout, ctx) == -1)
-                       logerr("%s: eloop_timeout_add_sec", __func__);
-               eloop_enter(ctx->ps_eloop);
+       ctx->options |= DHCPCD_EXITING;
+       if (eloop_timeout_add_sec(ctx->ps_eloop, PS_PROCESS_TIMEOUT,
+           ps_process_timeout, ctx) == -1)
+               logerr("%s: eloop_timeout_add_sec", __func__);
+       eloop_enter(ctx->ps_eloop);
 
 #ifdef HAVE_CAPSICUM
-               struct ps_process *psp;
-
-               TAILQ_FOREACH(psp, &ctx->ps_processes, next) {
-                       if (psp->psp_pfd == -1)
-                               continue;
-                       if (eloop_event_add(ctx->ps_eloop, psp->psp_pfd,
-                           ELE_HANGUP, ps_processhangup, psp) == -1)
-                               logerr("%s: eloop_event_add pfd %d",
-                                   __func__, psp->psp_pfd);
-               }
-#endif
+       struct ps_process *psp;
 
-               waited = eloop_start(ctx->ps_eloop, &ctx->sigset);
-               if (waited != EXIT_SUCCESS) {
-                       logerr("%s: eloop_start", __func__);
-                       error = waited;
-               }
-               eloop_timeout_delete(ctx->ps_eloop, ps_process_timeout, ctx);
+       TAILQ_FOREACH(psp, &ctx->ps_processes, next) {
+               if (psp->psp_pfd == -1)
+                       continue;
+               if (eloop_event_add(ctx->ps_eloop, psp->psp_pfd,
+                   ELE_HANGUP, ps_processhangup, psp) == -1)
+                       logerr("%s: eloop_event_add pfd %d",
+                           __func__, psp->psp_pfd);
        }
+#endif
 
-       if (IN_PRIVSEP_SE(ctx) && ctx->ps_root != NULL) {
-               struct ps_process *psp = ctx->ps_root;
-
-               if (ps_root_unlink(ctx, ctx->pidfile) == -1)
-                       logerr("%s: ps_root_unlink", __func__);
+       error = eloop_start(ctx->ps_eloop, &ctx->sigset);
+       if (error != EXIT_SUCCESS)
+               logerr("%s: eloop_start", __func__);
 
-               /* We cannot log the root process exited before we
-                * log dhcpcd exits because the latter requires the former.
-                * So we just log the intent to exit. */
-               logdebugx("%s%s%s will exit from PID %d",
-                   psp->psp_ifname,
-                   psp->psp_ifname[0] != '\0' ? ": " : "",
-                   psp->psp_name, psp->psp_pid);
-       }
+       eloop_timeout_delete(ctx->ps_eloop, ps_process_timeout, ctx);
 
        return error;
 }

diff --git a/src/privsep.h b/src/privsep.h
index 2f0304a7bf74a850123ec56781e010656a27d4b8..d19a8c430697b3414e4b53dd3e5eacba01ea6f76 100644 (file)
--- a/src/privsep.h
+++ b/src/privsep.h
@@ -109,14 +109,6 @@
 
 #define        PS_PROCESS_TIMEOUT      5       /* seconds to stop all processes */
 
-/* We always have ourself as a process */
-#define        PS_WAITING_FOR_PROCESSES(_ctx)                          \
-       ((IN_PRIVSEP_SE((_ctx)) &&                              \
-         TAILQ_LAST(&(_ctx)->ps_processes, ps_process_head) != (_ctx)->ps_root) ||     \
-         (!IN_PRIVSEP_SE((_ctx)) &&                            \
-         TAILQ_FIRST(&(_ctx)->ps_processes) !=                 \
-         TAILQ_LAST(&(_ctx)->ps_processes, ps_process_head)))
-
 #if defined(PRIVSEP) && defined(HAVE_CAPSICUM)
 #define PRIVSEP_RIGHTS
 #endif
@@ -248,6 +240,7 @@ int ps_stopprocess(struct ps_process *);
 struct ps_process *ps_findprocess(struct dhcpcd_ctx *, struct ps_id *);
 struct ps_process *ps_findprocesspid(struct dhcpcd_ctx *, pid_t);
 struct ps_process *ps_newprocess(struct dhcpcd_ctx *, struct ps_id *);
+bool ps_waitforprocs(struct dhcpcd_ctx *ctx);
 void ps_process_timeout(void *);
 void ps_freeprocess(struct ps_process *);
 void ps_freeprocesses(struct dhcpcd_ctx *, struct ps_process *);