From 4627c5d80fd677531878a819d30ad9f6d385b50b Mon Sep 17 00:00:00 2001 From: Roy Marples Date: Tue, 6 Sep 2022 09:18:08 +0100 Subject: [PATCH] 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. --- src/privsep-root.c | 34 ++++++++++++++------- src/privsep.c | 74 ++++++++++++++++++++++------------------------ src/privsep.h | 9 +----- 3 files changed, 60 insertions(+), 57 deletions(-) diff --git a/src/privsep-root.c b/src/privsep-root.c index a8290a40..28b8f59f 100644 --- 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 5a437ae3..3db9f4fc 100644 --- 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 2f0304a7..d19a8c43 100644 --- 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 *); -- 2.47.2