Skip to content

exec: preserve signals across command fork #572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sairex53 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

exec: preserve signals across command fork #572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/exec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,12 @@ exec_setup(struct command_details *details, int intercept_fd, int errfd)
* If SELinux is enabled, run the command via sesh, otherwise
* execute it directly.
* If the exec fails, cstat is filled in with the value of errno.
* If mask is not NULL, it is the signal mask to restore after the
* signal handlers have been reset.
*/
void
exec_cmnd(struct command_details *details, int intercept_fd, int errfd)
exec_cmnd(struct command_details *details, sigset_t *mask, int intercept_fd,
int errfd)
{
debug_decl(exec_cmnd, SUDO_DEBUG_EXEC);

Expand All @@ -261,7 +264,13 @@ exec_cmnd(struct command_details *details, int intercept_fd, int errfd)
}
#endif /* HAVE_PTRACE_INTERCEPT */

/*
* Reset the signal handlers before unblocking signals so that a
* signal forwarded by the parent before execve(2) is not lost.
*/
restore_signals();
if (mask != NULL)
sigprocmask(SIG_SETMASK, mask, NULL);
if (exec_setup(details, intercept_fd, errfd) == true) {
/* headed for execve() */
#ifdef HAVE_SELINUX
Expand Down Expand Up @@ -473,7 +482,7 @@ sudo_execute(struct command_details *details,
*/
if (direct_exec_allowed(details)) {
if (!sudo_terminated(cstat)) {
exec_cmnd(details, -1, -1);
exec_cmnd(details, NULL, -1, -1);
cstat->type = CMD_ERRNO;
cstat->val = errno;
}
Expand Down
17 changes: 14 additions & 3 deletions src/exec_monitor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ mon_backchannel_cb(int fd, int what, void *v)
*/
static void
exec_cmnd_pty(struct command_details *details, bool foreground,
int intercept_fd, int errfd)
sigset_t *mask, int intercept_fd, int errfd)
{
volatile pid_t self = getpid();
debug_decl(exec_cmnd_pty, SUDO_DEBUG_EXEC);
Expand Down Expand Up @@ -396,7 +396,7 @@ exec_cmnd_pty(struct command_details *details, bool foreground,
/* Execute command; only returns on error. */
sudo_debug_printf(SUDO_DEBUG_INFO, "executing %s in the %s",
details->command, foreground ? "foreground" : "background");
exec_cmnd(details, intercept_fd, errfd);
exec_cmnd(details, mask, intercept_fd, errfd);

debug_return;
}
Expand Down Expand Up @@ -551,6 +551,7 @@ exec_monitor(struct command_details *details, bool foreground,
struct monitor_closure mc;
struct command_status cstat;
struct sigaction sa;
sigset_t set, oset;
int errsock[2];
debug_decl(exec_monitor, SUDO_DEBUG_EXEC);

Expand Down Expand Up @@ -621,10 +622,19 @@ exec_monitor(struct command_details *details, bool foreground,
*/
init_exec_events_monitor(&mc, errsock[0]);

/*
* Block signals across the fork; the child unblocks them once it
* has reset the handlers (see exec_cmnd()) so that a signal
* delivered to it before execve(2) is not lost.
*/
sigfillset(&set);
sigprocmask(SIG_BLOCK, &set, &oset);

mc.cmnd_pid = sudo_debug_fork();
switch (mc.cmnd_pid) {
case -1:
sudo_warn("%s", U_("unable to fork"));
sigprocmask(SIG_SETMASK, &oset, NULL);
#ifdef HAVE_SELINUX
if (ISSET(details->flags, CD_RBAC_ENABLED)) {
if (selinux_restore_tty() != 0)
Expand All @@ -637,12 +647,13 @@ exec_monitor(struct command_details *details, bool foreground,
close(backchannel);
close(errsock[0]);
/* setup tty and exec command */
exec_cmnd_pty(details, foreground, intercept_fd, errsock[1]);
exec_cmnd_pty(details, foreground, &oset, intercept_fd, errsock[1]);
if (send(errsock[1], &errno, sizeof(int), 0) == -1)
sudo_warn(U_("unable to execute %s"), details->command);
_exit(EXIT_FAILURE);
/* NOTREACHED */
}
sigprocmask(SIG_SETMASK, &oset, NULL);
close(errsock[1]);
if (intercept_fd != -1)
close(intercept_fd);
Expand Down
13 changes: 9 additions & 4 deletions src/exec_nopty.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,9 +615,11 @@ exec_nopty(struct command_details *details,
/* Allocate and set signal events and the error pipe event.*/
init_exec_events(&ec, evbase, errpipe[0]);

/* Restore signal mask now that signal handlers are setup. */
sigprocmask(SIG_SETMASK, &oset, NULL);

/*
* Signals stay blocked across the fork; the child unblocks them
* once it has reset the handlers (see exec_cmnd()) so that a
* signal forwarded to it before execve(2) is not lost.
*/
ec.cmnd_pid = sudo_debug_fork();
switch (ec.cmnd_pid) {
case -1:
Expand Down Expand Up @@ -647,7 +649,7 @@ exec_nopty(struct command_details *details,
close(io_pipe[STDERR_FILENO][0]);
close(io_pipe[STDERR_FILENO][1]);
}
exec_cmnd(details, intercept_sv[1], errpipe[1]);
exec_cmnd(details, &oset, intercept_sv[1], errpipe[1]);
while (write(errpipe[1], &errno, sizeof(int)) == -1) {
if (errno != EINTR)
break;
Expand All @@ -658,6 +660,9 @@ exec_nopty(struct command_details *details,
}
sudo_debug_printf(SUDO_DEBUG_INFO, "executed %s, pid %d", details->command,
(int)ec.cmnd_pid);

/* Restore signal mask now that the command has been forked. */
sigprocmask(SIG_SETMASK, &oset, NULL);
/* Close the other end of the pipes and socketpairs. */
if (io_pipe[STDIN_FILENO][0] != -1)
close(io_pipe[STDIN_FILENO][0]);
Expand Down
22 changes: 13 additions & 9 deletions src/signal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,20 @@ restore_signals(void)
struct signal_state *ss;
debug_decl(restore_signals, SUDO_DEBUG_MAIN);

/*
* Restore all saved signals, not just the ones flagged for restore.
* The handlers sudo installed are inherited across fork(2) and a
* signal that arrives before execve(2) would be swallowed by them.
* The saved state is what execve(2) would leave: SIG_IGN or SIG_DFL.
*/
for (ss = saved_signals; ss->signo != -1; ss++) {
if (ss->restore) {
sudo_debug_printf(SUDO_DEBUG_INFO,
"restoring handler for signal %d: %s", ss->signo,
ss->sa.sa_handler == SIG_IGN ? "SIG_IGN" :
ss->sa.sa_handler == SIG_DFL ? "SIG_DFL" : "???");
if (sigaction(ss->signo, &ss->sa, NULL) != 0) {
sudo_warn(U_("unable to restore handler for signal %d"),
ss->signo);
}
sudo_debug_printf(SUDO_DEBUG_INFO,
"restoring handler for signal %d: %s", ss->signo,
ss->sa.sa_handler == SIG_IGN ? "SIG_IGN" :
ss->sa.sa_handler == SIG_DFL ? "SIG_DFL" : "???");
if (sigaction(ss->signo, &ss->sa, NULL) != 0) {
sudo_warn(U_("unable to restore handler for signal %d"),
ss->signo);
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/sudo_exec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ union sudo_token_un {

/* exec.c */
struct stat;
void exec_cmnd(struct command_details *details, int intercept_fd, int errfd);
void exec_cmnd(struct command_details *details, sigset_t *mask,
int intercept_fd, int errfd);
void terminate_command(pid_t pid, bool use_pgrp);
bool sudo_terminated(struct command_status *cstat);
void free_exec_closure(struct exec_closure *ec);
Expand Down