From 1032030f85524110244c777793ac3d07c65aed7c Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Sat, 8 Feb 2025 16:12:06 -0700 Subject: [PATCH] Split the code to fill an exec closure into two functions. This lets us initialize the exec closure early and fill in the events later. It also makes things consistent with the exec_pty version. --- src/exec_monitor.c | 31 +++++++++++++++++++++++-------- src/exec_nopty.c | 36 +++++++++++++++++++++++------------- src/exec_pty.c | 1 + 3 files changed, 47 insertions(+), 21 deletions(-) diff --git a/src/exec_monitor.c b/src/exec_monitor.c index 148cd1d8f..3f899ab43 100644 --- a/src/exec_monitor.c +++ b/src/exec_monitor.c @@ -407,22 +407,34 @@ exec_cmnd_pty(struct command_details *details, sigset_t *mask, } /* - * Fill in the monitor closure and setup initial events. - * Allocates read events for the signal pipe, error pipe and backchannel. + * Fill in the non-event part of the monitor closure. */ static void -fill_exec_closure_monitor(struct monitor_closure *mc, +init_exec_closure_monitor(struct monitor_closure *mc, const struct command_details *details, struct command_status *cstat, - int errfd, int backchannel) + int backchannel) { - debug_decl(fill_exec_closure_monitor, SUDO_DEBUG_EXEC); + debug_decl(init_exec_closure_monitor, SUDO_DEBUG_EXEC); /* Fill in the non-event part of the closure. */ + memset(mc, 0, sizeof(*mc)); mc->details = details; mc->cstat = cstat; mc->backchannel = backchannel; mc->mon_pgrp = getpgrp(); + debug_return; +} + +/* + * Fill in the monitor closure and setup initial events. + * Allocates read events for the signal pipe, error pipe and backchannel. + */ +static void +init_exec_events_monitor(struct monitor_closure *mc, int errfd) +{ + debug_decl(init_exec_events_monitor, SUDO_DEBUG_EXEC); + /* Setup event base and events. */ mc->evbase = sudo_ev_base_alloc(); if (mc->evbase == NULL) @@ -437,7 +449,7 @@ fill_exec_closure_monitor(struct monitor_closure *mc, sudo_fatal("%s", U_("unable to add event to queue")); /* Event for forwarded signals via backchannel. */ - mc->backchannel_event = sudo_ev_alloc(backchannel, + mc->backchannel_event = sudo_ev_alloc(mc->backchannel, SUDO_EV_READ|SUDO_EV_PERSIST, mon_backchannel_cb, mc); if (mc->backchannel_event == NULL) sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory")); @@ -542,7 +554,7 @@ int exec_monitor(struct command_details *details, sigset_t *oset, bool foreground, int backchannel, int intercept_fd) { - struct monitor_closure mc = { 0 }; + struct monitor_closure mc; struct command_status cstat; struct sigaction sa; int errsock[2]; @@ -578,6 +590,9 @@ exec_monitor(struct command_details *details, sigset_t *oset, goto bad; } + /* Fill in exec closure after creating a new session. */ + init_exec_closure_monitor(&mc, details, &cstat, backchannel); + /* * The child waits on the other end of a socketpair for the * parent to set the controlling terminal. It also writes @@ -648,7 +663,7 @@ exec_monitor(struct command_details *details, sigset_t *oset, * Create new event base and register read events for the * signal pipe, error pipe, and backchannel. */ - fill_exec_closure_monitor(&mc, details, &cstat, errsock[0], backchannel); + init_exec_events_monitor(&mc, errsock[0]); /* Restore signal mask now that signal handlers are setup. */ sigprocmask(SIG_SETMASK, oset, NULL); diff --git a/src/exec_nopty.c b/src/exec_nopty.c index 4de17fb08..8f9f32fa5 100644 --- a/src/exec_nopty.c +++ b/src/exec_nopty.c @@ -201,19 +201,17 @@ signal_cb_nopty(int signo, int what, void *v) debug_return; } - /* - * Fill in the exec closure and setup initial exec events. - * Allocates events for the signal pipe and error pipe. + * Fill in the non-event part of the exec closure. */ static void -fill_exec_closure(struct exec_closure *ec, struct command_status *cstat, - struct command_details *details, const struct user_details *user_details, - struct sudo_event_base *evbase, int errfd) +init_exec_closure(struct exec_closure *ec, struct command_status *cstat, + struct command_details *details, const struct user_details *user_details) { - debug_decl(fill_exec_closure, SUDO_DEBUG_EXEC); + debug_decl(init_exec_closure, SUDO_DEBUG_EXEC); /* Fill in the non-event part of the closure. */ + memset(ec, 0, sizeof(*ec)); ec->sudo_pid = getpid(); ec->ppgrp = getpgrp(); ec->cstat = cstat; @@ -221,6 +219,18 @@ fill_exec_closure(struct exec_closure *ec, struct command_status *cstat, ec->rows = user_details->ts_rows; ec->cols = user_details->ts_cols; + debug_return; +} + +/* + * Allocate and set events for the signal pipe and error pipe. + */ +static void +init_exec_events(struct exec_closure *ec, struct sudo_event_base *evbase, + int errfd) +{ + debug_decl(init_exec_events, SUDO_DEBUG_EXEC); + /* Setup event base and events. */ ec->evbase = evbase; @@ -543,7 +553,7 @@ exec_nopty(struct command_details *details, { int io_pipe[3][2] = { { -1, -1 }, { -1, -1 }, { -1, -1 } }; int errpipe[2], intercept_sv[2] = { -1, -1 }; - struct exec_closure ec = { 0 }; + struct exec_closure ec; sigset_t set, oset; debug_decl(exec_nopty, SUDO_DEBUG_EXEC); @@ -554,6 +564,9 @@ exec_nopty(struct command_details *details, if (policy_init_session(details) != true) sudo_fatalx("%s", U_("policy plugin failed session initialization")); + /* Fill in exec closure. */ + init_exec_closure(&ec, cstat, details, user_details); + /* * We use a pipe to get errno if execve(2) fails in the child. */ @@ -659,11 +672,8 @@ exec_nopty(struct command_details *details, if (ISSET(details->flags, CD_SET_TIMEOUT)) alarm(details->timeout); - /* - * Fill in exec closure, allocate event base, signal events and - * the error pipe event. - */ - fill_exec_closure(&ec, cstat, details, user_details, evbase, errpipe[0]); + /* Allocate and set signal events and the error pipe event. */ + init_exec_events(&ec, evbase, errpipe[0]); if (ISSET(details->flags, CD_INTERCEPT|CD_LOG_SUBCMDS)) { int rc = 1; diff --git a/src/exec_pty.c b/src/exec_pty.c index 8bfb83187..fb384d13c 100644 --- a/src/exec_pty.c +++ b/src/exec_pty.c @@ -971,6 +971,7 @@ init_exec_closure(struct exec_closure *ec, struct command_status *cstat, debug_decl(init_exec_closure, SUDO_DEBUG_EXEC); /* Fill in the non-event part of the closure. */ + memset(ec, 0, sizeof(*ec)); ec->sudo_pid = sudo_pid; ec->ppgrp = ppgrp; ec->cmnd_pid = -1;