Fix handling of signal forwarding when running commands in a script.

We need to forward signals from a process in the same pgrp if the
pgrp leader is not either sudo or the command itself.
This commit is contained in:
Todd C. Miller
2022-11-07 14:51:41 -07:00
parent c7071f6da0
commit 36742deec3
4 changed files with 43 additions and 32 deletions

View File

@@ -282,12 +282,14 @@ mon_signal_cb(int signo, int what, void *v)
* reboot that call kill(-1, SIGTERM) to kill all other processes. * reboot that call kill(-1, SIGTERM) to kill all other processes.
*/ */
if (USER_SIGNALED(sc->siginfo) && sc->siginfo->si_pid != 0) { if (USER_SIGNALED(sc->siginfo) && sc->siginfo->si_pid != 0) {
pid_t si_pgrp = getpgid(sc->siginfo->si_pid); pid_t si_pgrp;
if (sc->siginfo->si_pid == mc->cmnd_pid)
debug_return;
si_pgrp = getpgid(sc->siginfo->si_pid);
if (si_pgrp != -1) { if (si_pgrp != -1) {
if (si_pgrp == mc->cmnd_pgrp) if (si_pgrp == mc->cmnd_pgrp)
debug_return; debug_return;
} else if (sc->siginfo->si_pid == mc->cmnd_pid) {
debug_return;
} }
} }
deliver_signal(mc, signo, false); deliver_signal(mc, signo, false);

View File

@@ -122,6 +122,7 @@ signal_cb_nopty(int signo, int what, void *v)
struct sudo_ev_siginfo_container *sc = v; struct sudo_ev_siginfo_container *sc = v;
struct exec_closure *ec = sc->closure; struct exec_closure *ec = sc->closure;
char signame[SIG2STR_MAX]; char signame[SIG2STR_MAX];
pid_t si_pgrp;
debug_decl(signal_cb_nopty, SUDO_DEBUG_EXEC); debug_decl(signal_cb_nopty, SUDO_DEBUG_EXEC);
if (ec->cmnd_pid == -1) if (ec->cmnd_pid == -1)
@@ -151,38 +152,40 @@ signal_cb_nopty(int signo, int what, void *v)
case SIGQUIT: case SIGQUIT:
case SIGTSTP: case SIGTSTP:
/* /*
* Only forward user-generated signals not sent by a process in * Only forward user-generated signals not sent by a process other than
* the command's own process group. Signals sent by the kernel * the command itself or a member of the command's process group (but
* may include SIGTSTP when the user presses ^Z. Curses programs * only when either sudo or the command is the process group leader).
* often trap ^Z and send SIGTSTP to their own pgrp, so we don't * Signals sent by the kernel may include SIGTSTP when the user presses
* want to send an extra SIGTSTP. * ^Z. Curses programs often trap ^Z and send SIGTSTP to their own
* process group, so we don't want to send an extra SIGTSTP.
*/ */
if (!USER_SIGNALED(sc->siginfo)) if (!USER_SIGNALED(sc->siginfo))
debug_return; debug_return;
if (sc->siginfo->si_pid != 0) { if (sc->siginfo->si_pid != 0) {
pid_t si_pgrp = getpgid(sc->siginfo->si_pid); if (sc->siginfo->si_pid == ec->cmnd_pid)
if (si_pgrp != -1) {
if (si_pgrp == ec->ppgrp || si_pgrp == ec->cmnd_pid)
debug_return;
} else if (sc->siginfo->si_pid == ec->cmnd_pid) {
debug_return; debug_return;
si_pgrp = getpgid(sc->siginfo->si_pid);
if (si_pgrp != -1) {
if (si_pgrp == ec->cmnd_pid || si_pgrp == ec->sudo_pid)
debug_return;
} }
} }
break; break;
default: default:
/* /*
* Do not forward signals sent by a process in the command's process * Do not forward signals sent by the command itself or a member of the
* group, as we don't want the command to indirectly kill itself. * command's process group (but only when either sudo or the command is
* For example, this can happen with some versions of reboot that * the process group leader). We don't want the command to indirectly
* call kill(-1, SIGTERM) to kill all other processes. * kill itself. For example, this can happen with some versions of
* reboot that call kill(-1, SIGTERM) to kill all other processes.
*/ */
if (USER_SIGNALED(sc->siginfo) && sc->siginfo->si_pid != 0) { if (USER_SIGNALED(sc->siginfo) && sc->siginfo->si_pid != 0) {
pid_t si_pgrp = getpgid(sc->siginfo->si_pid); if (sc->siginfo->si_pid == ec->cmnd_pid)
if (si_pgrp != -1) {
if (si_pgrp == ec->ppgrp || si_pgrp == ec->cmnd_pid)
debug_return;
} else if (sc->siginfo->si_pid == ec->cmnd_pid) {
debug_return; debug_return;
si_pgrp = getpgid(sc->siginfo->si_pid);
if (si_pgrp != -1) {
if (si_pgrp == ec->cmnd_pid || si_pgrp == ec->sudo_pid)
debug_return;
} }
} }
break; break;
@@ -210,6 +213,7 @@ fill_exec_closure(struct exec_closure *ec,
debug_decl(fill_exec_closure, SUDO_DEBUG_EXEC); debug_decl(fill_exec_closure, SUDO_DEBUG_EXEC);
/* Fill in the non-event part of the closure. */ /* Fill in the non-event part of the closure. */
ec->sudo_pid = getpid();
ec->ppgrp = getpgrp(); ec->ppgrp = getpgrp();
ec->cstat = cstat; ec->cstat = cstat;
ec->details = details; ec->details = details;

View File

@@ -761,18 +761,21 @@ signal_cb_pty(int signo, int what, void *v)
break; break;
default: default:
/* /*
* Do not forward signals sent by a process in the command's process * Do not forward signals sent by the command itself or a member of the
* group, as we don't want the command to indirectly kill itself. * command's process group (but only when either sudo or the command is
* For example, this can happen with some versions of reboot that * the process group leader). We don't want the command to indirectly
* call kill(-1, SIGTERM) to kill all other processes. * kill itself. For example, this can happen with some versions of
* reboot that call kill(-1, SIGTERM) to kill all other processes.
*/ */
if (USER_SIGNALED(sc->siginfo) && sc->siginfo->si_pid != 0) { if (USER_SIGNALED(sc->siginfo) && sc->siginfo->si_pid != 0) {
pid_t si_pgrp = getpgid(sc->siginfo->si_pid); pid_t si_pgrp;
if (si_pgrp != -1) {
if (si_pgrp == ec->ppgrp || si_pgrp == ec->cmnd_pid) if (sc->siginfo->si_pid == ec->cmnd_pid)
debug_return;
} else if (sc->siginfo->si_pid == ec->cmnd_pid) {
debug_return; debug_return;
si_pgrp = getpgid(sc->siginfo->si_pid);
if (si_pgrp != -1) {
if (si_pgrp == ec->cmnd_pid || si_pgrp == ec->sudo_pid)
debug_return;
} }
} }
/* Schedule signal to be forwarded to the command. */ /* Schedule signal to be forwarded to the command. */
@@ -851,8 +854,9 @@ fill_exec_closure(struct exec_closure *ec, struct command_status *cstat,
debug_decl(fill_exec_closure, SUDO_DEBUG_EXEC); debug_decl(fill_exec_closure, SUDO_DEBUG_EXEC);
/* Fill in the non-event part of the closure. */ /* Fill in the non-event part of the closure. */
ec->cmnd_pid = -1; ec->sudo_pid = getpid();
ec->ppgrp = ppgrp; ec->ppgrp = ppgrp;
ec->cmnd_pid = -1;
ec->cstat = cstat; ec->cstat = cstat;
ec->details = details; ec->details = details;
ec->rows = user_details.ts_rows; ec->rows = user_details.ts_rows;

View File

@@ -74,6 +74,7 @@ struct exec_closure {
struct sudo_event *sigwinch_event; struct sudo_event *sigwinch_event;
struct command_status *cstat; struct command_status *cstat;
void *intercept; void *intercept;
pid_t sudo_pid;
pid_t monitor_pid; pid_t monitor_pid;
pid_t cmnd_pid; pid_t cmnd_pid;
pid_t ppgrp; pid_t ppgrp;